3Re: HTML::Template, CGI - concatenating strings & variables

In your case, all you really need to do make sure that the id the user submits is valid.

my $rpt_id = $CGI->param('rpt_id');

# trim any leading or trailing whitespace
$rpt_id =~ s/^\s*//;
$rpt_id =~ s/\s*$//;

# assuming report id is suppose to only contain digits
unless ($rpt_id =~ /^\d+$/) {
    # handle error - id contains more than digits
}
[download]

is just one example of "untainting" your paramaters that are submitted by someone (who could be trying to crack your CGI script). I recommend adding the taint switch to your "shebang" line:

#!/usr/local/bin/perl5_8 -T
[download]

Since you have already untainted $rpt_id by making it part of $rpt_tmpl like so:

my $rpt_tmpl = "cnc1_rpt" . $rpt_id . "_summary.tmpl";

# another way to achieve the same result:
my $rpt_tmpl = "cnc1_rpt@{[$rpt_id]}_summary.tmpl";

# and yet anther way
my $rpt_tmpl = sprintf("cnc1_rpt%d_summary.tmpl", $rpt_id);
[download]

you shouldn't have to worry about devious folks getting at other files like you would with the following DANGEROUS code:

my $file = $CGI->param('file');
open FH, '<', "$PATH/$file";
[download]

Even though you supply the path, the user can still submit something like ../../../etc/passwd ... bad.

Your code appears safe enough as it is, but ... it's still a good idea to make sure that what you let the user to submit is restricted.

jeffa

L-LL-L--L-LL-L--L-LL-L--
-R--R-RR-R--R-RR-R--R-RR
B--B--B--B--B--B--B--B--
H---H---H---H---H---H---
(the triplet paradiddle with high-hat)

Comment on 3Re: HTML::Template, CGI - concatenating strings & variables Select or Download Code

Replies are listed 'Best First'.
Re: 3Re: HTML::Template, CGI - concatenating strings & variables by Lori713 (Pilgrim) on Nov 17, 2003 at 20:15 UTC
I see what you mean by error checking first. As it turns out, the $rpt_id variable is actually set by the value of the radio button that is clicked; the user doesn't actually do anything but click on the radio button to indicate which report they want. I am especially appreciative of your help/comments/suggestions about how to improve my code with regards to security. This is a big concern to me since I'm new at this. I've already received an agreement from our dba's to review my code for security holes after I get my initial draft completed, but it's nice to get the holes filled before showing it to them! Thanks! Lori	[reply]
5Re: HTML::Template, CGI - concatenating strings & variables by jeffa (Bishop) on Nov 17, 2003 at 20:20 UTC
"... the user doesn't actually do anything but click on the radio button to indicate which report they want. Even though you "narrow the choices" on the interface, the user doesn't have to use the interface. Instead they could submit a GET query directly: `# contrived example http://foo.com/cgi-bin/form.cgi?rpt_id=../../../etc/password` [download] or use a web bot, etc. Even though 99% of the people don't know about this, the 1% that does is 100% of the devious people you need to worry about. ;) Cheers :) jeffa L-LL-L--L-LL-L--L-LL-L-- -R--R-RR-R--R-RR-R--R-RR B--B--B--B--B--B--B--B-- H---H---H---H---H---H--- (the triplet paradiddle with high-hat)	[reply] [d/l]
Re: 5Re: HTML::Template, CGI - concatenating strings & variables by Lori713 (Pilgrim) on Nov 17, 2003 at 20:38 UTC
I've tried putting "-T" on the shebang line, but then I get error messages saying it can't "find HTML::Template in @INC blah blah blah..." Did I misunderstand how to set up taint checking in the .pl scripts? Lori	[reply]
Re: Re: 3Re: HTML::Template, CGI - concatenating strings & variables by jgallagher (Pilgrim) on Nov 17, 2003 at 20:26 UTC
Keep in mind that just because you have radio buttons laid out with specific id numbers doesn't mean the user has to actually use those; there's nothing preventing them from manually setting rcpt_id to "/etc/passwd", for example. In short, never, ever trust user input; always check it as shown above.	[reply]