Danger!

Your script makes some very classic mistakes and has some very serious security vulnerabilities that could allow any person to write files anywhere on your system.

Some things to look at:

• use strict;
• use warnings;
• use CGI or die;
• Sanitizing user-provided path/filenames
• perlop
• perlsec

To literally answer the question you're asking, yes, you can put variables in a double-quoted string like that. It should do what you're expecting. So the problem must lie in how those variables are assigned. Some debugging code (e.g. print statements here and there) to check that each variable has the value you're expecting, and that each directory exists as you expect, etc., would help in your debugging efforts.

But I wouldn't use this logic as it is at all without doing some serious sanitizing of input and/or verification of each parameter they're passing (e.g. ensuring that the class is a valid class name, that the user is a valid user, etc.). Anyone can put "../../../" in any of those variables and back-track their way out of your filesystem.

#!/usr/local/bin/perl -wT

use strict;
use CGI;

my $q = CGI->new();

print $q->header();

# Using CGI is even easier than doing it yourself,
# so PLEASE(!) use it!
$studentid = $q->param('studentid');
$CSC = $q->param('CSC');
chomp($CSC);  
$assignment = $q->param('assignment');
$file = $q->param('file');
[download]

my $path = join ('/',
                   $HOME, 'classes', $CSC,
                   $studentid, $assignment,
                   'outputfile.txt');
[download]

# An example of "validated" input
my ($studentid) = $q->param('studentid') =~ /(\w+)/;
[download]

• Not having to write your own "text/html" header.
• Not having to test for various methods (POST vs. GET)
• Not having to worry about de-mangling %-ified parameters
• Not having to handle more than one instance of the same parameter name (i.e. a "CHECKBOX" with one-or-more selections possible)

Hi, tadman. You say, "Instead of using string interpolation, such as "$HOME/$x/$y", you should just join".

Next to doing dumb things, I hate doing wise things without knowing why.** And in this case I can't think of a reason why join would be better for this.

What is the reason for your assertion?

---------------------
**I've heard this called "Cargo Cult Correctness", i.e. (blindly) doing the right thing but not knowing the reason. ;-)

I think tadman ment 2 things. First of all, with the join method you only have a hard-coded separator in one place. If you want another, just replace that one, or better, define a variable for that.

Two, you'd better assign variables, or arrays, to define the subdirectories node for node. That way, the code gets more readible (less comments needed) and better maintainable.

Jeroen

tainted, or icky

heh. amused. You realise I'll now be forced to think of tainted data as 'icky'. ;)

andy.

A suggestion: use strict warnings and diagnostics or die; it also would be a good idea to use the module CGI, as its parser is considered the best for query strings; note also that your if statements can be combined. I'm not sure, but I believe it's the roll-your-own parser that's causing you grief. Also look at perlsec. Try using CGI, strict, and warnings (or -w), as well as possibly taint mode (perlrun / perlsec) then come back if you still have problems.

open( OUTFILE, ">>$filename" ) or die ( "no go miss ");

with


open( OUTFILE, ">>$filename" ) or die ( "no go miss: $!");
[download]

Hi,
Thought that comes to mind. how does perl deal with appending to a file that doesn't exist? I'd have thought it just created it, but I'm not sure.

From perldoc -f open:
If the filename begins with '>>', the file is opened for appending, again being created if necessary.

I was curious too and just thought I'd share my findings. Quinn Slack
perl -e 's ssfhjok qupyrqs&&tr sfjkohpyuqrspitnrapj"hs&&eval&&s&&&'

One alternative (to constructing the pathname by untainting user-supplied parameters) would be to match the incoming parameters against a hash of allowable values. Obviously this only works if there are a fixed set of allowable values.

So, say that the incoming param is called 'dir', and you don't want to allow anything except 'a','b' or 'c', you could do:

my %allowed_dirs = (a => 'a', b => 'b', c => 'c');
my $dir = $allowed_dirs{$q->param('dir')};
die "go away, you nasty horrid hacker" unless defined $dir;
[download]

hth,
andy.

my $file = $FORM{file};
$file =~ s{.*/}{}; # remove any path, this takes care of the ../.. sec
+urity problem
$file =~ s/[^\w\-.]//g; # remove anything in the filename that isn't a
+ word character, a dash, or a dot. This takes care of the `rm -rf /` 
+type threats.
...
[download]

Type path of document to upload or click "Browse" to navigate to docum
+ent:
<INPUT TYPE="file" name="path">
[download]

#!/usr/local/bin/perl -w
# 2010c.pl

use CGI;
require ("cgi-lib.pl");

my $q = CGI->new();

print $q->header();

$studentid = $q->param('studentid');
$CSC = $q->param('CSC');
chomp($CSC);  
$assignment = $q->param('assignment');
$file = $q->param('file');

$HOME = (getpwnam("wfp51916"))[7];
$filename = "$HOME/classes/$CSC/$studentid/$assignment/outputfile.txt"
+;
open( OUTFILE, ">>$filename" ) or die ( "no go miss ");
print OUTFILE "$studentid\n";
close( OUTFILE );

print "$studentid<br>";        
print "$CSC<br>";
print "$assignment<br>";
#below is the variable I cannot get to print
print "$file<br>";
[download]

lfindle, Welcome to Perlmonks and Perl, and we all sympathize with your feeling:

While I realize I made a lot of progress today in my quest to learn some Perl, I'm left with the
feeling that I have many more questions than when I began.

They often say in science that asking one question finding the answer and finally knowing does not
salve one's need for understanding, but raises one's need to know more.

Or as better said by Matt Cartmill, the anthropologist:

"As an adolescent I aspired to lasting fame, I craved factual certainty, and I thirsted for a meaningful vision of human life -
so I became a scientist. This is like becoming an archbishop so you can meet girls."

Cheers, and I hope you enjoy your time here! :)

The only thing I haven't seen mentioned, which it looks like you need, is to check that the directory exists. Presumably your code is not just supposed to work for Laura, so unless you are anticipating all the required directory and subdirectory names, you will need to create them on the fly.

Small snippet:
print "Directory $dirname does".((-d $dirname)?" ":" not ")."exist.\n";

<a href="http://www.graq.co.uk">Graq</a>