Re: PHP variable sanitization functions

[Jamie Pratt <jamie () nucdc org>]

Liam Quinn wrote:

> On Sun, 24 Aug 2003, Gavin Zuchlinski wrote:

<snip>

> Your sanitize_system_string function allows the string to contain 
> newlines and quotation marks.  An attacker could close the argument with a 
> quotation mark, add a newline, and then run an arbitrary command.

am I missing something?  - isn't newline \n ? - isn't that a \n in 
there, as well as the quotation marks? (")

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

> You're also missing some other shell metacharacters.  Here's the list of
> shell metacharacters given in the WWW Security FAQ [*]:
>
> &;`'\"|*?~<>^()[]{}$\n\r
>
> [*] http://www.w3.org/Security/faq/wwwsf4.html#CGI-Q7

ok so I suck at regexp (it's still on the to-do list when I have 
time...) but some of those appear to be in the above code as well? - but 
anyhoo - here's my contribution to add to sanitize_system_string: (noted 
with + ... arguments/comments welcome - ie, am I being redundant 
anywhere? These could also apply to the other functions I imagine, but 
I'm afraid of being redundant - maybe a base sanitizer function would 
help - one in which each would be called first regardless of data type 
to clean - put generic stuff like strip_tags(), trim(), utf8_decode(), 
etc.. ?)

anyways, here's my changes to sanitize_system_string for a little extra 
security: (untested as of yet, but some parts are used on another site I 
run without problems.)

// sanitize a string in prep for passing a single argument to system() 
(or similar)
function sanitize_system_string($string)
{
  $pattern = '/(;|\||`|>|<|&|^|"|'."\n|\r|'".'|{|}|[|]|\)|\()/'; // no 
piping, passing possible environment variables ($),
                           // seperate commands, nested execution, file 
redirection,
                           // background processing, special commands 
(backspace, etc.), quotes
                           // newlines, or some other special characters
  $replacement = "";
  $string = preg_replace($pattern, $replacement, $string);

  $pattern = '/\$/';
  $replacement = '\\\$';
  $string = preg_replace($pattern, $replacement, $string);

+ // trim leading and trailing whitespace if any
+ $string = trim($string);
+ //remove all accented chars
+ $string = strtr($string, 
"ŠŒŽšœžŸ¥µÀÁÂÃÄÅÆÇÈÉÊËÌÍÎÏÐÑÒÓÔÕÖØÙÚÛÜÝßàáâãäåæçèéêëìíîïðñòóôõöøùúûüýÿ",

"SOZsozYYuAAAAAAACEEEEIIIIDNOOOOOOUUUUYsaaaaaaaceeeeiiiionoooooouuuuyy");
+       // utf8_decode() converts the ISO-8859-1 characters in a string 
encoded with the Unicode UTF-8 encoding to single-byte ASCII chars - XSS 
attacks sometimes use unicode to hide the attack string.
+      $string = utf8_decode($string);
+      //  remove any HTML and PHP tags if they exist
+      $string = strip_tags($string);
+      // how i get rid of backticks and ;'s using str_replace
+      $string = str_replace("`", "", "$string");
+      $string = str_replace(";", "", "$string");

  return '"'.$string.'"'; //make sure this is only interpretted as ONE 
argument
}

I don't have a fix for the double-quote issue as I haven't had time to 
test today - anyone else?


regards,
jamie