Beefy Boxes and Bandwidth Generously Provided by pair Networks
Your skill will accomplish
what the force of many cannot
 
PerlMonks  

Web form security

by earthboundmisfit (Chaplain)
on Jul 30, 2001 at 17:03 UTC ( #100845=perlquestion: print w/ replies, xml ) Need Help??
earthboundmisfit has asked for the wisdom of the Perl Monks concerning the following question:

In reading over the node, Executing a function on submit, I realized that one of my coding practices has some potential security risks.

Instead of retrieving the value of parameter values on an as-needed basis, I quite typically dump the entire contents of param() to scalars, as in:

use strict; use warnings; use CGI; my $q = new CGI; @names = $q->param; foreach $name(@names){ $$name=$q->param($name); #print $name, ': ', $$name, '<BR>'; }
So any form field in the HTML file automatically becomes a Perl var in the cgi script when the form is submitted.

After spending some time on this site, it occurs to me that some unscrupulous but savvy user could quite easily break a program by "hijacking" my form, i.e. downloading a local copy and modifying the field names in the form and changing the ACTION statement to a fully qualified URI.

How ruined would my day be if the following were submitted?:

<FORM METHOD=POST ACTION="http://mydomain.com/cgi-bin/search.cgi"> <INPUT TYPE='text' name='a'> <INPUT TYPE='text' name='b'> <INPUT TYPE='text' name='/'> <INPUT TYPE='text' name='\'> <INPUT TYPE='text' name='!'> <INPUT TYPE='text' name='_'> </FORM>
I understand that a lot depends on the particulars of my script, but my generalized question is is my concern warranted or is there some sort of built in security measure in CGI.pm to handle reserved variable names? If my concern is warranted, has anyone out there come up with a bullet proof way of dealing with this?

I've read over the CGI docs and can't find anything related to this topic. (??)

Comment on Web form security
Select or Download Code
Re: Web form security
by Masem (Monsignor) on Jul 30, 2001 at 17:18 UTC
    When writing anything bound for CGI use, you should invoke it with the -T (taint) option, as it will tell you what data has been presented by the user, and which operations will be potentally bad (such as what you have above).

    CGI.pm has no provisions for this, as it highly recommends staying to the param() function.

    A better solution in your case is to create an array of allowed variable names, and then only read variable names from the form that are in this group, eg:

    my @safe_vars = qw( name address phone age ); ... @names = $q->param; foreach $name(@names){ if ( grep { $_ eq $name } @safe_vars ) { $$name=$q->param($name); #print $name, ': ', $$name, '<BR>'; } }
    (TMTOWTDI, of course, but the general idea is there).

    -----------------------------------------------------
    Dr. Michael K. Neylon - mneylon-pm@masemware.com || "You've left the lens cap of your mind on again, Pinky" - The Brain

Re: Web form security
by tachyon (Chancellor) on Jul 30, 2001 at 17:39 UTC

    I think you are being overly concerned. There is a big diference between eval "$name" and $$name = $q->param($name). As for your concers run this and see what happens, it emulates all your CGI input:

    @names = qw(a b / \ ! _); # check we have all the input worries print @names; print "\n"; # and so??? foreach $name(@names){ print $name, ': ', $$name, "<BR>\n"; }

    cheers

    tachyon

    s&&rsenoyhcatreve&&&s&n.+t&"$'$`$\"$\&"&ee&&y&srve&&d&&print

      There's still danger in this, even if it's not through perl internals.

      Say I have variable $chargecustomerscard which, later in the code, which actually does the monetary processing in my script. Normally, this variable is only set after the status is determined from a database query, cookie checks, etc.

      Certainly this variable name is sufficiently hidden from the end user since they can't see the script. But if a more malicious user were to discover that this variable exists, and that one is populating the variable space by the methods used here, then that user could easily "overwrite" the correct value of that variable just by generating the right URL request, and cause numerous problems.

      It's nearly always better to explicitly define what you are looking for than to exclude the cases you don't want to look for when it comes to security in this fashion.

      -----------------------------------------------------
      Dr. Michael K. Neylon - mneylon-pm@masemware.com || "You've left the lens cap of your mind on again, Pinky" - The Brain

Re: Web form security
by Beatnik (Parson) on Jul 30, 2001 at 17:47 UTC
    That's a contradiction... Web <-> Security. =))

    Greetz
    Beatnik
    ... Quidquid perl dictum sit, altum viditur.
Re: Web form security
by premchai21 (Curate) on Jul 30, 2001 at 18:36 UTC
    Generally, symbolic references are not a good idea, especially when combined with names you get from outside! Also, I don't know how this is passing strict -- @names is undeclared, and if I'm not mistaken strict 'refs' disables symbolic references. Try instead using a hash (also added CGI functions rather than raw HTML, and string interpolation)...
    use strict; use warnings; use CGI; my $q = CGI->new; my @names = $q->param; my %param; foreach my $name (@names) { $param{$name} = $q->param($name); print "$name: $param{$name}", $q->br; }
    Or, more concisely:
    use strict; use warnings; use CGI; my $q = CGI->new; my %params = map { $_, $q->param($_) } ($q->param); print join $q->br, map { "$_: $params{$_}" } ($q->param);
      Ok, you caught me. I wasn't using strict in my "real" script. Forgive me. I'm weak =)

      I'm still not 100% clear on what I should or should not be doing, but I understand a little better what's at stake. Thanks.

        A little more explanation:
        use strict; use warnings; use CGI;
        Use strict and -w, except of course for one-liners and/or short throwaway scripts. Note that the warnings pragma only works under Perl v5.6.0+.
        my $q = CGI->new; my @names = $q->param; my %param;
        This instantiates the CGI object and fills @names with the parameter list, then declares %param, to be used later.
        foreach my $name (@names)
        Iterating over each parameter in order,
        { $param{$name} = $q->param($name);
        Set the value in the parameter hash ($param{$name}) to the parameter value ($q->param($name))...
        print "$name: $param{$name}", $q->br; }
        ... and print it; the $name and $param{$name} values are interpolated into the string. $q->br just generates an empty BR tag.

        The other version:

        use strict; use warnings; use CGI; my $q = CGI->new;
        Same as before.
        my %params = map { $_, $q->param($_) } ($q->param);
        This simultaneously instantiates %params and fills it with, for each element in $q->param (the parameter list), the name ($_, the placeholder variable -- see map / perlvar) plus the value ($q->param($_)). When this is put into a hash these pairs turn into keys and values.
        print join $q->br, map { "$_: $params{$_}" } ($q->param);
        This first takes the parameter list ($q->param) and maps each element to the string "$_: $params{$_}" which is the name ($_) plus a colon, space, and value ($params{$_}) -- accessing an element of the params hash with key being the name. Then it joins these strings together with the empty BR tag, and prints the result. Hope this helps.
(ichimunki) Re: Web form security
by ichimunki (Priest) on Jul 30, 2001 at 18:51 UTC
    Why do this at all, even if it isn't a security issue? FWIW, some of these will simply crash the program, others will depend on whether you directly use the special variables (like $_ for implied $_ as in print if /match/;) or indirectly (like $/ for file reading).

    If the forged form contains a name that you have not pre-declared this will simply crash the script, but for these built-in vars you do have an issue. But why even write your CGI so that it can crash so easily? Since you have to predeclare all your valid param names for $$param to even work, why not simply use my $this = param( 'this' ); in the first place? That way you can introduce default values or error detection and detainting all at the same time.
    my $this = text_detaint( param('this') ) || 'default text'; or my $this = num_detaint( param('this') ) or output_error( "this", "must be a number" );
importing CGI parameters (boo)
by boo_radley (Parson) on Jul 30, 2001 at 18:59 UTC
    Your method of importing variables is maybe not the best way to go about things. from CGI's POD :
    IMPORTING ALL PARAMETERS INTO A NAMESPACE: $query->import_names('R'); This creates a series of variables in the 'R' namespace. For example, $R::foo, @R:foo. For keyword lists, a variable @R::keywords will appear. If no namespace is given, this method will assume 'Q'. WARNING: don't import anything into 'main'; this is a major security risk!!!! In older versions, this method was called import(). As of version 2.20, this name has been removed completely to avoid conflict with the built-in Perl module import operator.
    And, looking in CGI's source code (v. 2.74), we see the following inside of import_name :
    # protect against silly names ($var = $param)=~tr/a-zA-Z0-9_/_/c; $var =~ s/^(?=\d)/_/;
    which will only check the validity of the parameter's name; its value may still be malicious in some fashion. See perlsec for more on dealing with this type of 'tainted' data.
Re: Web form security
by jlongino (Parson) on Jul 30, 2001 at 23:38 UTC
    For some very general information about Cross-Site Scripting you might want to check this out. It is not Perl oriented, but may be useful.

Log In?
Username:
Password:

What's my password?
Create A New User
Node Status?
node history
Node Type: perlquestion [id://100845]
Approved by root
help
Chatterbox?
and the web crawler heard nothing...

How do I use this? | Other CB clients
Other Users?
Others browsing the Monastery: (8)
As of 2014-08-30 19:15 GMT
Sections?
Information?
Find Nodes?
Leftovers?
    Voting Booth?

    The best computer themed movie is:











    Results (293 votes), past polls