As you may have noticed by my intro, most of the above code is not mine. And, I did miss several rather glaring misconceptions in the original. Mucho thanks for pointing those out. Doing a code audit of another's work is something very new to me.

This "lib" appears to be used almost exclusively by folks trying to accept simple forms information. Which means almost zero cleanup happens after the fact. Hence anything passed back from this routine is used almost as is.

And to tell you the truth I never expected anyone opening an upload (enctype="multipart/form-data") on the web would use such a lame lib as subparseform.lib. Surely one would jump to more sophisticated scripts by that time.

My original intention was to see if a plug-n-play replacement for subparseform.lib could be made which would not break their existing scripts relying upon the lib.

Is this helpling the weak stay weak? I'm not sure. I was just so annoyed to see the original subparseform.lib proliferating even further onto the net with such lame security.

Maybe there's a way to replace subparseform.lib with a CGI.pm enabled version that also creates the %formdata hash. This might give them a version which will still feed into their existing code AND gives them the all important jump start into using CGI.pm?

I'm hoping something can be done short of leave the security hole ladden lib alone and leave these misguided webdevelopers to their own devices.

I'll let ya know. And thanks again for the input.

Claude

The following is just a rough stab at what you are looking for. The problem with handling the security in the "Parse_Form" routine is that security considerations will differ from application to application. Trying to embed the security directly into a form-parsing routine doesn't work. As security concerns change from application to application, the parsing routine would have to be adjusted. You either have multiple programs affected by the change or multiple copies of the form parsing routine.

Because I understand where you're heading with this, I'll make a suggestion. I do not recommend the following code, but perhaps it would let people new to CGI programming see how easy it is to use CGI.pm. Please note that the following code is untested.

sub Parse_Form {
    use CGI qw/:standard/;

    foreach my $key ( param() ) {
        my @vals = param( $key );
        if ( scalar @vals == 1 ) {
            $formdata{ $key } = $vals[0];
        } elsif ( ! @vals ) {
            $formdata{ $key } = undef;
        } else {
            # This should be adjusted as necessary.
            $formdata{ $key } = \@vals;
        }
    }
}
[download]

Actually, one can get all of the form data with a map:

%formdata = map { $_, [ param( $_ ) ] } param;
[download]

The main problem there is how one gets a single value for the param "foo":

my $val = $formdata{ 'foo' }->[0];
[download]

I think too many programmers would balk at the awkward syntax.

Note how "multiple values" are handled in my original routine. Properly, multiple values that map to a single key should be represented in an array. Unfortunately, as you've discovered, many insist upon joining with a comma/space or with an ASCII zero (\0). How that final one is handled depends upon how to preserve current functionality of their programs.

Xxaxx asked:

Is this helping the weak stay weak?

Yes. Personally, I have strong objections to teaching people bad coding practices and then gradually bringing them up to speed. They wind up with many bad habits to unlearn. The reality, however, is that this is often the situation we have to deal with.

I will confess that I am unsure about how you're going about this, but that's probably because I haven't given it a lot of thought. If you must take this approach, I think the snippet above is probably a good start. If you want to more accurately duplicate the functionality of the original scripts, perhaps you should consider adding a standard HTML parsing module to look for SSIs or HTML comments. Then, you can strip them out (don't do a regex - they're trickier than you might think). However, this is going back to forcing your notion of security on them. That's not good practice, as mentioned above.

Good luck!

Cheers,
Ovid

Update: I forgot to mention some issues with your approach to security:

my $allowedCHARS = 'a-zA-Z0-9\!\"\#\$\%\&\'\(\)\*\+\,\-\.\/\:\;\<\=\>\
+?\@\[\\\]\^\_\`\{\|\}\~';
[download]

What the heck is that? Well, lets clean it up a bit.

my $allowedCHARS = 'a-zA-Z0-9' . quotemeta( '!"#$%&\'()*+,-./:;<=>?@[\
+]^_`{|}~' );
[download]

Well, that clears it up and shows some serious problems. If you are going to hold their hand, hoping they don't get burnt, why even allow them to play with firecrackers? I see escapes, backticks, pipes, and dots, all of which are extremely common sources of security holes. Depending upon what is done with the rest of the characters (and on what OS they are running), there are still plenty of other issues here. I do like the fact that you state specifically what is allowed -- that's much better than stating what isn't allowed -- but it's still extremely permissive, depending upon what you want to do.

Security concerns simply vary too much from site to site. It is not possible to address them all in a form parsing routine. That's a limitation that you'll just have to accept and it is incumbent upon all of us to raise the awareness of these issues.

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

FYI: CGI.pm has a backwards compatability layer to cgi-lib.pl which can be invoked with

use CGI;
CGI::ReadParse();
[download]

This creates the %in hash that cgi-lib.pl (a deprecated lib) folks liked to see. If you still insist on making your own POST/GET parser, why not just cut and paste the CGI parsing routine found in "sub new{...}" and tweak as necessary?

But why do you wish to propagate bad code? Just yell at them to use CGI! There is NO excuse!


AgentM Systems nor Nasca Enterprises nor Bone::Easy nor Macperl is responsible for the comments made by AgentM. Remember, you can build any logical system with NOR.