Re: Perl variable sanitization functions

[Tim Greer <chatmaster () charter net>]

On Fri, 2003-08-29 at 10:22, Gavin Zuchlinski wrote:
> I translated the variable sanitization functions I wrote in PHP over to Perl, 
> you can check them out at http://libox.net/sanitize.php. The Perl functions 
> are the same in functionality as their PHP counterpart functions. Send me an 
> email if Im missing something or the module doesnt work (I just translated it 
> quickly now).

In sanitize_paranoid_string, some other characters are likely safe, such
as the underscore character.  Rather than using:

$string =~ s/[^a-zA-Z0-9]//g;

Try:

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

# Any non word character.  tr/// would likely be better here.


I see you're accepting the arguments as sub($string, $min, $len), but I
don't see the point to the last two on a string in the manner you are
using them?

What's ($min != "") about?? != on a string comparison???  What are you
doing there?  I'm confused?

($len < $min) <-  A 'less than' operator on two potentially undefined
variables?  Why are you character comparing strings with < ?  What
purpose does this function serve?  Did you mean cmp?  And, if so, what
would the intention of that condition be?

Same for (($max != "") && ($len > $max)))... I don't understand the
intent on a character string.  I don't think this is doing that you
think it's doing.

In sanitize_system_string, this has the same issues as mentioned above,
an in addition, this seems wasteful:

$string =~ s/(;|\||`|>|<|&|^|"|'."\n|\r|'".'|{|}|[|]|\)|\()//g;

If you're going to strip out the characters you don't want, this is a
bad design in logic, you should decide only on the things you want to
allow, rather than these alternatives to strip out (and you might
consider tr/// if that's the case anyway). Also, you are capturing the
match, why?  You check check out ?: to ensure you don't save $1 since
you don't need it. This would be better written as:

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

Or maybe $string =~ s/[^\W\-whatever]//g;

As above, since all the other characters are stripped anyway, from what
I can tell.

if((($min != '') && ($len < $min)) || (($max != '') && ($len > $max)))

Same issues as in other sub routine outlined above.

In sanitize_sql_string tr/// would be better used.

if((($min != '') && ($len < $min)) || (($max != '') && ($len > $max)))

Same issue mentioned twice now.  Check out cmp, eq, ne, etc. for string
comparison.

In the sanitize_html_string sub routine, seems okay for the most part (I
didn't check to see if these could be used to actually create an
unsanitized HTML string (this is something to watch out for), but you
should be able to get by with just replacing < and > and " and '.  You
should have a global check for new lines, line feeds, etc. from the
initial check.  You should not need to check for nor replace -, +, etc.
for an HTML string, and replacing & would break hotlinks that should
otherwise not pose a risk if you disable HTML tags or inserting anything
into a hotlink/image, etc. field on input.

if((($min != '') && ($len < $min)) || (($max != '') && ($len > $max)))

Suffers from the same problems as mentioned before.

In sanitize_int, you have:

$int =~ s/[^0-9\-]//g;

This doesn't handle floating points ( I see you do that in a different
sub routine... I'm just curious why?), as well as decimals (i.e.
45.234), nor other types of actual numerical values such as 10e5,
-45.323e54 etc.  Also, check out \d or \D, depending on what and how you
check.

if((($min != '') # wrong.  Check for 0 w/ !=, or use ne for an empty
string.

&& ($int < $min)) # Okay, depending on the intention.

|| (($max != '') && ($int > $max)))  # same as above.

In sanitize_float, you use:

$float =~ s/[^0-9.\-]//g;

This would allow for non real numbers, such as
....-----0935...346..-0-.9346846...--3-3-5-3....5. to pass through. 
Check for \d or \D depending.

 if((($min != '') && ($float < $min)) || (($max != '') && ($float >
$max)))

Same issues mentioned above in other sub routine.

I didn't have time the time to really go over this, but just noted these
issues upon glancing at it.  If I get time later, I'll try and provide
more details.  I like what you guy's are doing, but I'm curious to know
if there isn't already a module existing that provides these checks?  If
not, it's great to see you guy's have the initiative to get some out
there! :-)

Regards,
Tim Greer