Beefy Boxes and Bandwidth Generously Provided by pair Networks
XP is just a number
 
PerlMonks  

taint check that I thought worked

by jcpunk (Friar)
on Feb 02, 2004 at 16:45 UTC ( [id://325903]=perlquestion: print w/replies, xml ) Need Help??

jcpunk has asked for the wisdom of the Perl Monks concerning the following question:

I thought this taint check worked on usernames composed of letters or numbers from 1-16 characters, but apartently it does not. Anyone have any thoughts as to why?
lib.pl ------------- package lib; use vars '%common'; %common = ( 'username' => " " ); sub untaint_username { my $tainted = shift; # patern match letters and numbers for 1-16 characters (inclusive) $tainted =~ s/\W//g; # remove all non word characters if ($tainted =~ /\A([a-zA-Z0-9]{1-16})\z/i) { return $1; } # behold a username else {print "failure\n";} } 1;
----------------- prog.pl ----------------- #!/usr/bin/perl -wT use strict; require './lib.pl'; $lib::common{username}="unixhelp"; $lib::common{username}=lib::untaint_username($lib::common{username});
I dont think that anything weird is going on, so I guess the expression doesnt match what I think it does.

jcpunk
all code is tested, and doesn't work so there :p (varient on common PM sig for my own ammusment)

Replies are listed 'Best First'.
Re: taint check that I thought worked
by cbro (Pilgrim) on Feb 02, 2004 at 17:03 UTC
    I'm pretty sure you just overlooked this...in the line
    $tainted =~ /\A([a-zA-Z0-9]{1-16})\z/i

    I think you wanted a comma in the {1,16}.

    HTH,
    Chris
Re: taint check that I thought worked
by hardburn (Abbot) on Feb 02, 2004 at 19:12 UTC

    Your question as asked is answered, but I wanted to make another note:

    $tainted =~ s/\W//g; # remove all non word characters if ($tainted =~ /\A([a-zA-Z0-9]{1,16})\z/i)

    Basically, these two lines together say you can have everything in \w match (assuming an ASCII-clean input) except the underscore (and that 'i' at the end of the match statment is useless). Underscores are usually harmless, so it might be better to simply write:

    if( $tainted =~ /\A ( \w{1,16} ) \z/x ) {

    Which also has the advantage that if someone inputs 'firstname&lname&&' (which won't pass the length test if the '&' remain), it will spit it back as "failure" instead of doing something with the untainted version (which will be 'firstnamelname', and would pass the length test).

    ----
    I wanted to explore how Perl's closures can be manipulated, and ended up creating an object system by accident.
    -- Schemer

    : () { :|:& };:

    Note: All code is untested, unless otherwise stated

Re: taint check that I thought worked
by welchavw (Pilgrim) on Feb 02, 2004 at 17:04 UTC

    I think you want \Z, not \z. Also, you want {1,16}, not {1-16}.

    ,welchavw

      \Z (upper-case) matches either at the end of the string or just before a newline at the end of the string (just like $ normally does; but $'s behaviour changes with the /m flag, \Z always stays the same).

      \z (lower-case) matches only at the end of the string, and is probably what he (assuming for the moment that "punk" implies "male") wants.

        In this case, I belive that \z and \Z would be equivalent, as \W chars (including \n's) are stripped just before the matching. Am I right?

Re: taint check that I thought worked
by ysth (Canon) on Feb 02, 2004 at 17:05 UTC
    You want {1,16}, not {1-16}
Re: taint check that I thought worked
by Fletch (Bishop) on Feb 02, 2004 at 17:06 UTC
    • ITYM {1,16}; see perldoc perlre
    • Not directly related to the question at hand, but all lowercase package names are usually reserved for pragmatic modules (such as strict or lib).
Re: taint check that I thought worked
by Not_a_Number (Prior) on Feb 02, 2004 at 19:35 UTC
    if ($tainted =~ /\A([a-zA-Z0-9]{1-16})\z/i)

    My immediate thought was, if I wanted to check the length of a string, I would use the built-in length() function.

    My second thought was, as this line

    $tainted =~ s/\W//g;

    removes all non-word characters, why bother checking for them all again except one (the underscore) in the next line? Especially as the sub as it stands will return, say, 'NotaNumber' if it's fed with 'Not-a-Number' but will print 'failure' if it gets 'Not_a_Number'. Is this what you really want?

    My third thought was that even if your sub prints 'failure', it returns 1 (assuming that the print is successful), which by your definition ('usernames composed of letters or numbers from 1-16 characters') is a valid username :-)

    Maybe something like this would be better:

    sub untaint_username { my $str = shift; $str =~ s/[\W_]//g; # remove all non word characters (incl. unders +core) if ( $str and length $str < 17 ) { return $str; } else { return 'failure!'; } }

    Note the exclamation mark in return 'failure!', which is there solely to allow you to deal with the remote possibility that one of your users might decide to call themselves 'failure'...

    dave

    Update: Just realised that, by your definition, '0' (zero) is also a valid username. Change the if... line to:

       if ( defined $str and length $str < 17 ) {

    Update 2: Better still:

       if ( $str and defined $str and length $str < 17 ) {

Log In?
Username:
Password:

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

How do I use this?Last hourOther CB clients
Other Users?
Others romping around the Monastery: (9)
As of 2024-03-28 10:17 GMT
Sections?
Information?
Find Nodes?
Leftovers?
    Voting Booth?

    No recent polls found