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> Link to comment https://forums.phpfreaks.com/topic/202410-paranoid-secure-html-form/ 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. Link to comment https://forums.phpfreaks.com/topic/202410-paranoid-secure-html-form/#findComment-1061241 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); } } } Link to comment https://forums.phpfreaks.com/topic/202410-paranoid-secure-html-form/#findComment-1061331 Share on other sites More sharing options...
Jiin Posted May 21, 2010 Author Share Posted May 21, 2010 Bueller..... Link to comment https://forums.phpfreaks.com/topic/202410-paranoid-secure-html-form/#findComment-1061655 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. Link to comment https://forums.phpfreaks.com/topic/202410-paranoid-secure-html-form/#findComment-1061661 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); ?> Link to comment https://forums.phpfreaks.com/topic/202410-paranoid-secure-html-form/#findComment-1061673 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. Link to comment https://forums.phpfreaks.com/topic/202410-paranoid-secure-html-form/#findComment-1062206 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... Link to comment https://forums.phpfreaks.com/topic/202410-paranoid-secure-html-form/#findComment-1062209 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 Link to comment https://forums.phpfreaks.com/topic/202410-paranoid-secure-html-form/#findComment-1062211 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); Link to comment https://forums.phpfreaks.com/topic/202410-paranoid-secure-html-form/#findComment-1062245 Share on other sites More sharing options...
Recommended Posts
Archived
This topic is now archived and is closed to further replies.