Well,

    $ENV{ PATH }  = '/bin:/usr/bin';
        # or whatever you need
    $ENV{ SHELL } = '/bin/sh' if exists $ENV{SHELL};
[download]

I'd expect a CGI::Safe module to untaint parameters (defaulting to only /(\w[-\w.]+)/ but allowing alternates). Based on what this does, I see no point in having CGI::Safe to inherit from CGI.pm. I'd rather move the new() functionality into import() and have your module work via:

use CGI qw( :standard );
use CGI::Safer( POST_MAX=>1024*1024 );
my $q= CGI->new();
[download]

Update: I'm not suggesting that people should use both the OO and functional interfaces to CGI.pm at the same time. I'm just giving examples of the two cases in one short bit of example code.

tye wrote:

my system doesn't have a /bin/sh nor /bin nor /usr/bin. I hate modules that want you to edit the global module source code to provide reasonable behavior.

That bothered me, too, since I also don't have those on my system. I tried to think of how to set those based on OS, but I know that there are far too many operating systems out there for me to consider all of the possibilities. I also considered simply deleting the SHELL and PATH variables. This might be reasonable because, in any event, the programmer should explicitly set those when working with CGI programs. I simply provided useful defaults that would work for many programmers. Perhaps simply deleting them would be better.

You comment about needing to edit the module source misses the point, I think. There is no need to edit the source since, if I'm not mistaken, all the programmer needs to do is explicitly set the PATH and SHELL prior to needing to use them. The programmer can do this in his or her code with no need to edit the source. Since these variables are tainted, this is a huge security hole if these are not explicitly set from within the program. This module is primarily a utility for newer CGI programmers who may not be aware of these issues and it should make their programming environment a bit safer.

Interesting idea about untainting parameters. I'll have to give that some thought and consider it for a future release as an option. I don't want to have it automatically untaint anything (I think that's what you meant) as this is intended to be a drop-in replacement for CGI. If I untaint parameters for the programmer, this could break a lot of existing code. Of course, that might be a good thing considering some of the code we see out there. Naturally, though, if someone understands all of the issues that this module addresses, they probably don't need it :)

As a side note, Lincoln Stein stated that this module was "sensible" and, while I don't want to quote him without his permission, he intends to modify the docs to point to it and possibly include it in the CGI bundle. I think this module is a good compromise between security and useability. Many, many good programmers who have nice taint checking and good security practices in their CGI programs still forget to disable uploads, delete unsafe environment variables, and whatnot. This is merely a catch-all for those things they should be doing anyway.

Update: I forgot to thank you for the feedback: Thanks :)

Also, at least one other monk has commented that they like CGI::Safer as the namespace. I'll have to give that some thought as "Safe" might be stretching it. Hmm...

Cheers,
Ovid

Join the Perlmonks Setiathome Group or just click on the the link and check out our stats.

No, I'd have to edit the module source code because the proper settings for $ENV{PATH} and $ENV{SHELL} are set in one place, my web server, and I have no desire to have 20 CGI scripts with separate, probably out-of-date copies of that configuration information.

If I'm writing a set-UID script, then tainting is correct to tell me that I shouldn't trust PATH nor SHELL (because the user can override these). If you have horrid web server management such that you can't trust these values, then I think your problem is simply "horried web server management". (:

Well, if you want a drop-in replacement, then I'd certainly move away from the @ISA= qw(CGI), change the name, and have users simply add one line to their scripts rather than changing two lines of their scripts.

Please don't use the name "CGI::Safe" unless you address the number one safety problem of CGI: blindly trusting data that comes in over the internet. That is, the "untainting" of CGI parameters. Now, doing that would also justify the @ISA= qw(CGI) design.

The old-style param() method usage could untaint the default way that I described but either a new method or new arguments to param() would allow the coder to specify a regex or subroutine that untaints the data and the module would provide useful templates for common parameter types (like relative paths that don't allow "..").

Actually, I prefer the second option (extending your module). I think it would be great if using CGI::Safe prevented you from ever getting a parameter that didn't match /^\w[-\w.]*\z/ unless you gave it a different specification for that parameter. That would probably do a world of good toward improving CGI code. It would also make using -T easier and would mean that not using -T is no longer a major security error. :)

        - tye (but my friends call me "Tye")