http://qs321.pair.com?node_id=135908


in reply to cgi.pm file upload script

Desipte your impressions, this set of scripts is written with the older cgi-lib.pl library (as mentioned by grep) and a handrolled form parser (update: and, of course, not the reccommend CGI module). (Why include a library that does something and do it again? Beats me.)

More importantly, these scripts contain potentially serious security holes. They often use form input directly from the web browser in forming filenames which are then written to or deleted. This means that through these scripts someone could potentially overwrite or delete any file your script has access to. Even worse, this script will, under some circumstances (update: the hole grep found will probably allow that most of the time. I found another (`echo "$body" | $mail ...`) which would probably not be so common, being dependent on the setting of what mailer to use), include user input as part of a shell command. This means that someone could probably even run arbitrary shell commands on the system (e.g. rm -rf / to remove all files the script can remove).

As for the coding style, these scripts are similarly bad. The idention is horribly inconsistent. They don't use warnings and strict, let alone taint checking. They don't check the return value of many system calls (e.g. open, unlink). Variables are "declared" with both local and my -- only one should be used, ideally my, since this script intends to run on perl5 systems (as evidenced by the perl5-only use statement). (because my wasn't introduced till perl5, local was typically used in the same way in perl4 which is at least 8 years out of date.)

This program also uses syntax like this for prototypes:

sub sendmailer($recipient, $sender, $subject, $message){
This syntax is not supported in any written version of perl.

update: Elaborating on a point grep made, requiring 777 permissions could also be considered a security flaw: it lets pretty much anyone else on the web server you are using mess with the files this script is managing. In almost any way they want to.