Please read the 'perlsec' man page.
open(LINKPAGE, $in{html});
This is one of the worst things you can do in a CGI script. I can pass an argument of html=id;cat+/etc/passwd| to your script, or even more evilly, html=rm+-rf+/| or html=>/etc/passwd or all sorts of evil things.
You should a) strip out any strange characters; b) verify that the item in $in{html} refers to a filename in an appropriate location; and c) open it with something like open(LINKPAGE, "< $in{html}");
When writing CGI scripts, always keep perlsec in mind and always run with 'taint checking' enabled (-T). This would have spotted the fact that $in{html} is not safe to trust in critical calls like open() or system(). | [reply] [d/l] |
I won't tear the code apart, but I highly suggest you do NOT use cgi-lib.pl, but use CGI.pm instead. It does have a 'mode' where you can still use the methods from cgi-lib.pl.
Also, there is a security concern here, as mentioned in another reply. Please take a look at perlsec, use -T (ALL CGI should use -T), and the Untaint.pm module on CPAN.
Cheers,
KM | [reply] |
$filename is being reduced without using File::Basename. In this case, the name could contain \n, so the replacement would stop too early,
allowing us to have a name with slashes in it. In fact, I think a name of
"\n|/your/command/here foo bar.gif"
or whatever $theext is set to would be opened just fine. Actually, I can see that there'd be a little work to get through the maze, but nonetheless, the wrong
cargo-cult code was used here, and that makes this code dangerous.
Also, the Location: header needs a space after the colon, required by RFC.
As a style issue, using File::Copy would be preferred.
-- Randal L. Schwartz, Perl hacker | [reply] [d/l] |
Humm... I didn't get part of the code. If you have:
$openbr="<";
$closebr=">";
and$wholepage=~s/</$openbr/g;
$wholepage=~s/>/$closebr/g;
aren't you just saying$wholepage=~s/</</g;
$wholepage=~s/>/>/g;
What good does that do?
I may be missing something here.
#!/home/bbq/bin/perl
# Trust no1!
| [reply] [d/l] [select] |
actually what it was supposed to be instead of
$openbr="<";
$closebr="&rt;";
but I think something happened. | [reply] [d/l] |