Jiin Posted May 20, 2010 Share Posted May 20, 2010 Hello All, env: php5 Linux I am concerned about XSS and co related vulnerabilites. I am using a alphanumberic "white list" technique. Is the form sufficient and are there any additional concerns/gotchas? The Code: ----------------------------- <?php function clean($input) { return preg_replace('/[^a-zA-Z0-9\s]/', '', $input); //only allows letters, numbers and spaces } ?> <!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Transitional//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd"> <html xmlns="http://www.w3.org/1999/xhtml" > <head> <title>Safe Form</title> </head> <body> <form action="" method="POST"> <input name="username" type="text" id="username"> <input name="password" type="password" id="password"> <input type="submit" name="Submit" value="Submit"> <input name="reset" type="reset" id="reset" value="Reset"> </form> <?php echobr(clean($_POST['username'])); echobr(clean($_POST['password'])); ?> </body> </html> Quote Link to comment Share on other sites More sharing options...
dpacmittal Posted May 20, 2010 Share Posted May 20, 2010 Hello All, env: php5 Linux I am concerned about XSS and co related vulnerabilites. I am using a alphanumberic "white list" technique. Is the form sufficient and are there any additional concerns/gotchas? The Code: ----------------------------- <?php function clean($input) { return preg_replace('/[^a-zA-Z0-9\s]/', '', $input); //only allows letters, numbers and spaces } ?> <!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Transitional//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd"> <html xmlns="http://www.w3.org/1999/xhtml" > <head> <title>Safe Form</title> </head> <body> <form action="" method="POST"> <input name="username" type="text" id="username"> <input name="password" type="password" id="password"> <input type="submit" name="Submit" value="Submit"> <input name="reset" type="reset" id="reset" value="Reset"> </form> <?php echobr(clean($_POST['username'])); echobr(clean($_POST['password'])); ?> </body> </html> I guess this would be quite enough. Quote Link to comment Share on other sites More sharing options...
Jiin Posted May 20, 2010 Author Share Posted May 20, 2010 So it occurred to me to loop through the $_POST associative array and then I thought maybe h@xors could force extra arrays into the $_POST making it a multi-dimensional array. If I understand correctly preg_replace doesn't operate on multi-dimensional arrays so then maybe this code will work. Gurus please confirm. //Designed to clean HTML form $_POST, note $_POST is normally a associative array //but could be manipulated to become a multi-dimensional array so why not clean both? //clean() function allow recursing; arrays passed by reference function clean(&$_POST) { foreach ($_POST as &$data) { if (!is_array($data)) { // If it's not an array, clean it //Allow only letters, numbers and spaces $data = preg_replace('/[^a-zA-Z0-9\s]/', '', $data); // addslashes(), mysql_real_escape_string() or whatever } else { // If it IS an array, keep calling the function on it until it is broken down clean($data); } } } Quote Link to comment Share on other sites More sharing options...
Jiin Posted May 21, 2010 Author Share Posted May 21, 2010 Bueller..... Quote Link to comment Share on other sites More sharing options...
phant0m Posted May 21, 2010 Share Posted May 21, 2010 function clean(&$_POST) { I have no idea what PHP makes of this.... $_POST is a global variable and I don't think you should use it as function parameter. Just change it to something else to make sure there are no complications. Quote Link to comment Share on other sites More sharing options...
Jiin Posted May 21, 2010 Author Share Posted May 21, 2010 Who was that masked man? That worked. Thank you so much! My white list holy grail of form input validation accomplished! Code adopted from: http://php.net/manual/en/control-structures.foreach.php The working code: --------------------------------------------- <?php //Designed to clean HTML form $_POST, note $_POST is normally a associative array //but could be manipulated to become a multi-dimensional array so why not clean both? //Also protects against $_POST key and value poisoning //clean() function allow recursing; arrays passed by reference to clean function and foreach loop function clean(&$array) { foreach ($array as &$data) { // If it's not an array, clean it if (!is_array($data)) { $data = preg_replace('/[^a-zA-Z0-9\s]/', '', $data); //Allow only letters, numbers and spaces but could addslashes(), mysql_real_escape_string() or whatever echo "$data<br />"; } else { // array not broken down yet so keep calling the function on it until it is broken down clean($data); } } } // Output clean($_POST); ?> Quote Link to comment Share on other sites More sharing options...
phant0m Posted May 23, 2010 Share Posted May 23, 2010 You're welcome. I would consider passing $_POST by value and returning a cleaned array instead of passing by reference. Just in case you need to have both the raw input and the cleaned one at the same time. Quote Link to comment Share on other sites More sharing options...
newbtophp Posted May 23, 2010 Share Posted May 23, 2010 you don't need to loop, use array_map() with the clean() function, which would apply it to all values/elements... Quote Link to comment Share on other sites More sharing options...
Jiin Posted May 23, 2010 Author Share Posted May 23, 2010 Most excellent advice gentlemen. I will implement and re-post code. FREAK OUT Quote Link to comment Share on other sites More sharing options...
phant0m Posted May 23, 2010 Share Posted May 23, 2010 Yes, array_map would be an option, but you will still need to check whether the element that array_map gives to your callback, i.e. 'clean' function is an array itself, and then call it recursively. You will also have to think about how you want your code to look: Compare: $cleaned = array_map('clean', $_POST); and $cleaned = clean($_POST); Quote Link to comment Share on other sites More sharing options...
Recommended Posts
Join the conversation
You can post now and register later. If you have an account, sign in now to post with your account.