Razor42 Posted July 24, 2013 Posted July 24, 2013 Hi there, I am currently creating a mod and have used quite alot of $_POST's around the mod and want to know how these can be secured and when they need to be secured. Small example of form....: echo "Fill in the form below to recruit a new hoe. It costs 1000 Gold coins to hire a hoe.<br /><br /> <form action='brothel.php?action=recruit_sub' method='post'> Name: <input type='text' name='hname' /><br /> Age: <input type='text' name='hage' /><br /> Skin Color: <input type='text' name='hcolor' /><br /> Speciality: <input type='text' name='hspec' /><br /> <input type='submit' value='Recruit' /></form><br /><br /> <a href='brothel.php'>Go back to brothel</a>"; Then entering into DB.... $db->query("INSERT INTO hoes VALUES('', $userid, '{$_POST['hname']}', '{$_POST['hage']}', 0, 1, '{$_POST['hspec']}', '{$_POST['hcolor']}')"); Is this insecure? If so why? How would it be secured? Thanks in advance. Quote
a_bertrand Posted July 24, 2013 Posted July 24, 2013 yes it's highly insecure. ANY string going to the SQL string must be escaped correctly or yet better use bindings (by using PDO or MySQLi). It doesn't matter if the value come from $_GET, $_POST, Cookies or whatever else. Quote
Razor42 Posted July 24, 2013 Author Posted July 24, 2013 I know that this could be done via mysql_real_escape_string but according to PHP.net this is deprecated as of PHP 5.5.0 so what alternative could I use? Quote
Dominion Posted July 24, 2013 Posted July 24, 2013 (edited) I know that this could be done via mysql_real_escape_string but according to PHP.net this is deprecated as of PHP 5.5.0 so what alternative could I use? The mysql_ extension has been deprecated. Mysqli or PDO are both alternatives. Edited July 24, 2013 by Dominion Quote
rockwood Posted July 25, 2013 Posted July 25, 2013 Mccodes db class is old (also not escaping values properly by Old MCC) so i think it is bit better mysql.class.txt Quote
Script47 Posted July 25, 2013 Posted July 25, 2013 (edited) Use the HTML number form for numbers: <input type='number' name=''> Validate form check if numbers are only inserted: if (!ctype_digit($_POST['number'])) { // Some error code } else { $number = $_POST['number']+0; // Credit to berty for telling me this. :) } $someTextVar = $db->escape(htmlentities($_POST['someTextVarForm'])); // escaping data, and htmlentities to change what HTML output would be - http://www.w3schools.com/php/func_string_htmlentities.asp $someTextVar = mysqli_real_escape_string($link, (htmlentities($_POST['someTextVarForm']))); // MySQLi version of the above Check to see if var isset/empty, I don't think this is security (please tell me if I'm wrong): if (!isset($_POST['someVar1']) || !isset($_POST['someVar2']) || (empty($_POST['someVar1']) || empty($_POST['someVar2']))) { echo '<br/>One or more of the required fields are empty please go back and try again.'; $h->endpage(); exit(); } This is merely some of the stuff possible, by no means am I a pro, just thought I would write some stuff I do. Hope it helps and good luck with your mod mate. :) Edited July 25, 2013 by Script47 Quote
Barrikor Posted July 25, 2013 Posted July 25, 2013 Use the HTML number form for numbers: <input type='number' name=''> I don't think it works below IE10.... also Firefox will interpret it the same as an input type=text Quote
Script47 Posted July 25, 2013 Posted July 25, 2013 I don't think it works below IE10.... also Firefox will interpret it the same as an input type=text Ah right, then that's why just in case you put the second alternative, but right I did not know that, will keep in mind. Thanks :) Quote
rockwood Posted July 25, 2013 Posted July 25, 2013 I don't think it works below IE10.... also Firefox will interpret it the same as an input type=text HTML5 having inbuilt validation Quote
rockwood Posted July 25, 2013 Posted July 25, 2013 Use the HTML number form for numbers: <input type='number' name=''> Validate form check if numbers are only inserted: if (!ctype_digit($_POST['number'])) { // Some error code } else { $number = abs(intval($_POST['number'])); // Turn in to variable so it easier to obtain and you don't have to use the $_POST[''], and get absolute value. } $someTextVar = $db->escape(htmlentities($_POST['someTextVarForm'])); // escaping data, and htmlentities to change what HTML output would be - http://www.w3schools.com/php/func_string_htmlentities.asp $someTextVar = mysqli_real_escape_string($link, (htmlentities($_POST['someTextVarForm']))); // MySQLi version of the above Check to see if var isset/empty, I don't think this is security (please tell me if I'm wrong): if (!isset($_POST['someVar1']) || !isset($_POST['someVar2']) || (empty($_POST['someVar1']) || empty($_POST['someVar2']))) { echo '<br/>One or more of the required fields are empty please go back and try again.'; $h->endpage(); exit(); } This is merely some of the stuff possible, by no means am I a pro, just thought I would write some stuff I do. Hope it helps and good luck with your mod mate. :) isset and empty both are different Quote
a_bertrand Posted July 25, 2013 Posted July 25, 2013 And it's useless if you think about security as you would wrongly think that only numbers (for example) can be sent instead the browser could still send other stuff for example from older browser versions or from people which do it on purpose. As first rule of security: never trust user input. Quote
rockwood Posted July 25, 2013 Posted July 25, 2013 if (!empty($_POST['name'])) { $_POST['name'] = filter_var($_POST['name'], FILTER_SANITIZE_STRING); }else{echo "Error";} i am sanitizing Quote
Barrikor Posted July 25, 2013 Posted July 25, 2013 if (!empty($_POST['name'])) { $_POST['name'] = filter_var($_POST['name'], FILTER_SANITIZE_STRING); }else{echo "Error";} i am sanitizing I wouldn't really call it sanitizing until you whitelist allowed chars, and also take unicode chars into account. Quote
Alan Posted July 25, 2013 Posted July 25, 2013 if (!ctype_digit($_POST['number'])) { // Some error code } else { $number = abs(intval($_POST['number'])); // Turn in to variable so it easier to obtain and you don't have to use the $_POST[''], and get absolute value. } A remarkable number of errors in so few lines; No existence testing - ie: array_key_exists('number', $_POST) abs() - Why ? ctype_digit() looks at the digits 0-9 only no - sign intval() - Why ? as above, ctype_digit() look sat the digits 0 -9 only - no decimal point Have you tried intval() on large numbers? The response various between 32 & 64 bit machines, and in reality, it is unlikely that you actually need an integer. SELECT username FROM users WHERE userid = "123" works just as well as SELECT username FROM users WHERE userid = "123" and while the 123 in this case is (in the former example) as string, you don't need to escape it as we already know it is injection safe. Quote
HauntedDawg Posted July 25, 2013 Posted July 25, 2013 Mccodes db class is old (also not escaping values properly by Old MCC) so i think it is bit better Surely you did not write that. And did you bother to check if it adapts to current mccode without changing all the CODE? Or bother to adapt the class to fit in mccode? Quote
Aventro Posted July 25, 2013 Posted July 25, 2013 Use the HTML number form for numbers: <input type='number' name=''> Validate form check if numbers are only inserted: if (!ctype_digit($_POST['number'])) { // Some error code } else { $number = abs(intval($_POST['number'])); // Turn in to variable so it easier to obtain and you don't have to use the $_POST[''], and get absolute value. } $someTextVar = $db->escape(htmlentities($_POST['someTextVarForm'])); // escaping data, and htmlentities to change what HTML output would be - http://www.w3schools.com/php/func_string_htmlentities.asp $someTextVar = mysqli_real_escape_string($link, (htmlentities($_POST['someTextVarForm']))); // MySQLi version of the above Check to see if var isset/empty, I don't think this is security (please tell me if I'm wrong): if (!isset($_POST['someVar1']) || !isset($_POST['someVar2']) || (empty($_POST['someVar1']) || empty($_POST['someVar2']))) { echo '<br/>One or more of the required fields are empty please go back and try again.'; $h->endpage(); exit(); } This is merely some of the stuff possible, by no means am I a pro, just thought I would write some stuff I do. Hope it helps and good luck with your mod mate. :) You should also remove the escaping (htmlentities) when inserting in database, it's ideal to have as raw data as possible and then simply do the escaping out in your presentation logic. So remove htmlentities to the variables being inserted in the database; and use them when outputting instead. Quote
Razor42 Posted July 25, 2013 Author Posted July 25, 2013 So I have been working on the system and have done some security on it.... The form still remains... echo "Fill in the form below to recruit a new hoe. It costs 1000 Gold coins to hire a hoe.<br /><br /> <form action='brothel.php?action=recruit_sub' method='post'> Name: <input type='text' name='hname' /><br /> Age: <input type='text' name='hage' /><br /> Skin Color: <input type='text' name='hcolor' /><br /> Speciality: <input type='text' name='hspec' /><br /> <input type='submit' value='Recruit' /></form><br /><br /> <a href='brothel.php'>Go back to brothel</a>"; And then.... if (empty($_POST['hname']) || empty($_POST['hage']) || empty($_POST['hcolor']) || empty($_POST['hspec'])) { echo "You have missed one or more fields. Please go back and try again.<br /> <a href='brothel.php?action=open'>Go back</a>"; $h->endpage(); exit; } if($ir['goldcoins'] < 1000) { echo "You don't have enough Gold Coins<br /> <a href='brothel.php'>Go back to brothel</a>"; $h->endpage(); exit; } echo "Your hoe was created<br /> <a href='brothel.php'>Go back to brothel</a>"; $_POST['hname'] = mysql_real_escape_string($_POST['hname']); $_POST['hage'] = (int)$_POST['hname']; $_POST['hspec'] = mysql_real_escape_string($_POST['hname']); $_POST['hcolor'] = mysql_real_escape_string($_POST['hname']); $db->query("INSERT INTO hoes VALUES('', $userid, '{$_POST['hname']}', '{$_POST['hage']}', 0, 1, '{$_POST['hspec']}', '{$_POST['hcolor']}')"); $db->query("UPDATE users SET goldcoins=goldcoins-1000 WHERE userid=$userid"); Quote
SRB Posted July 25, 2013 Posted July 25, 2013 So I have been working on the system and have done some security on it.... The form still remains... echo "Fill in the form below to recruit a new hoe. It costs 1000 Gold coins to hire a hoe.<br /><br /> <form action='brothel.php?action=recruit_sub' method='post'> Name: <input type='text' name='hname' /><br /> Age: <input type='text' name='hage' /><br /> Skin Color: <input type='text' name='hcolor' /><br /> Speciality: <input type='text' name='hspec' /><br /> <input type='submit' value='Recruit' /></form><br /><br /> <a href='brothel.php'>Go back to brothel</a>"; And then.... if (empty($_POST['hname']) || empty($_POST['hage']) || empty($_POST['hcolor']) || empty($_POST['hspec'])) { echo "You have missed one or more fields. Please go back and try again.<br /> <a href='brothel.php?action=open'>Go back</a>"; $h->endpage(); exit; } if($ir['goldcoins'] < 1000) { echo "You don't have enough Gold Coins<br /> <a href='brothel.php'>Go back to brothel</a>"; $h->endpage(); exit; } echo "Your hoe was created<br /> <a href='brothel.php'>Go back to brothel</a>"; $_POST['hname'] = mysql_real_escape_string($_POST['hname']); $_POST['hage'] = (int)$_POST['hname']; $_POST['hspec'] = mysql_real_escape_string($_POST['hname']); $_POST['hcolor'] = mysql_real_escape_string($_POST['hname']); $db->query("INSERT INTO hoes VALUES('', $userid, '{$_POST['hname']}', '{$_POST['hage']}', 0, 1, '{$_POST['hspec']}', '{$_POST['hcolor']}')"); $db->query("UPDATE users SET goldcoins=goldcoins-1000 WHERE userid=$userid"); Or maybe even set it to show you what you did wrong, in the errors? And check all data for conforming? <?php if ($_SERVER['REQUEST_METHOD'] == "POST") { $name = check_string($_POST['hname']); $spec = check_string($_POST['hspec']); $color = check_string($_POST['hcolour']); $age = check_int($_POST['hage']); if ($ir['goldcoins'] < 1000) { echo '<p>You do not have enough gold coins.</p>'; } else if ($name == FALSE) { echo '<p>You did not enter a name.</p>'; } else if ($spec == FALSE) { echo '<p>You did not enter a spec</p>'; } else if ($color == FALSE) { echo '<p>You did not enter a color.</p>'; } else if ($age == FALSE) { echo '<p>You did not enter an age.</p>'; } else { $sql = "INSERT INTO `hoes` VALUES ('', '{$ir['userid']}', '{$name}', '{$age}', 0, 1, '{$spec}', '{$color}')"; $db->query($sql); $sql = "UPDATE `users` SET `goldcoins` = `goldcoins` - 1000 WHERE `userid` = '{$ir['userid']}'"; $db->query($sql); echo '<p>Your hoe was created.</p>'; } echo '<p><a href="brothel.php">Go Back</a></p>'; } function check_int($input) { return (strlen($input) > 0 && ctype_digit($input)) ? $input : FALSE ; } function check_string($input) { global $c; return (strlen($input) > 0 && is_string($input)) ? mysql_real_escape_string($input, $c) : FALSE ; } NOTE This was wrote at 9:15am after being awake 15 minutes, so I'll probably end up changing something later. ~Luke Quote
Ben Nash Posted July 25, 2013 Posted July 25, 2013 Turn the form values into variables so your query would look like this: $db->query("INSERT INTO hoes VALUES('', $userid, '{$value}', '{$value}', 0, 1, '{$value}', '{$value}')"); Quote
sniko Posted July 25, 2013 Posted July 25, 2013 Turn the form values into variables so your query would look like this: $db->query("INSERT INTO hoes VALUES('', $userid, '{$value}', '{$value}', 0, 1, '{$value}', '{$value}')"); Add in the column names, so you won't have to heavily edit the script if you alter the table INSERT INTO hoes (`id`,`user`,`column`,`column`,`column`,`column`,`column`,`column`) VALUES('', $userid, '{$value}', '{$value}', 0, 1, '{$value}', '{$value}') Quote
HauntedDawg Posted July 25, 2013 Posted July 25, 2013 Add in the column names, so you won't have to heavily edit the script if you alter the table INSERT INTO hoes (`id`,`user`,`column`,`column`,`column`,`column`,`column`,`column`) VALUES('', $userid, '{$value}', '{$value}', 0, 1, '{$value}', '{$value}') In that CASE, whats the point of passing the `id` then? Assuming its auto_increment, it's not even needed in there.. jus sayin Quote
Script47 Posted July 25, 2013 Posted July 25, 2013 (edited) isset and empty both are different I know that? You should also remove the escaping (htmlentities) when inserting in database, it's ideal to have as raw data as possible and then simply do the escaping out in your presentation logic. So remove htmlentities to the variables being inserted in the database; and use them when outputting instead. I've been told to use htmlentities then when removing the data from database use html_entity_decode(), just saying. if (!ctype_digit($_POST['number'])) { // Some error code } else { $number = abs(intval($_POST['number'])); // Turn in to variable so it easier to obtain and you don't have to use the $_POST[''], and get absolute value. } A remarkable number of errors in so few lines; No existence testing - ie: array_key_exists('number', $_POST) abs() - Why ? ctype_digit() looks at the digits 0-9 only no - sign intval() - Why ? as above, ctype_digit() look sat the digits 0 -9 only - no decimal point Have you tried intval() on large numbers? The response various between 32 & 64 bit machines, and in reality, it is unlikely that you actually need an integer. SELECT username FROM users WHERE userid = "123" works just as well as SELECT username FROM users WHERE userid = "123" and while the 123 in this case is (in the former example) as string, you don't need to escape it as we already know it is injection safe. Ah sorry mate, as I said I ain't no pro, just trying to help out. One question, if I have ctype_digit() then I don't need the other stuff I added? Edited July 25, 2013 by Script47 Quote
jcvenom Posted July 25, 2013 Posted July 25, 2013 this is how you secure $_POST In the first case htmlspecialchars() probably is the best choice, allowing for users to use all characters like <, >, &, etc. In the second case you will need to use some database escaping function like mysql_real_escape_string or a prepared statement with PDO or mysqli. Prepared statements are the best choice here but if you are only familiar with mysql then mysql_real_escape_string works fine too. If you are not using mysql then there are similar functions in most SQL APIs. In the third case do both but separately, which gives you two diffrent results, one for output and one for database. this is how i do it below is references for you hope this helps References: http://php.net/manual/en/function.htmlspecialchars.php http://php.net/manual/en/function.mysql-real-escape-string.php http://php.net/manual/en/book.pdo.php http://php.net/manual/en/book.mysqli.php Quote
SRB Posted July 25, 2013 Posted July 25, 2013 this is how you secure $_POST In the first case htmlspecialchars() probably is the best choice, allowing for users to use all characters like <, >, &, etc. In the second case you will need to use some database escaping function like mysql_real_escape_string or a prepared statement with PDO or mysqli. Prepared statements are the best choice here but if you are only familiar with mysql then mysql_real_escape_string works fine too. If you are not using mysql then there are similar functions in most SQL APIs. In the third case do both but separately, which gives you two diffrent results, one for output and one for database. this is how i do it below is references for you hope this helps References: http://php.net/manual/en/function.htmlspecialchars.php http://php.net/manual/en/function.mysql-real-escape-string.php http://php.net/manual/en/book.pdo.php http://php.net/manual/en/book.mysqli.php Say what now? I've just gave a perfect example of how to input data. Htmlspecialchars for database input? Are you kidding me? Mysql_real_escape_string() is all that's required since it only escapes what it needs to and therefore keeps the data in the rawest possible form. If you really want to, you could add strip tags, but again, takes away from raw data. Output then needs wrapping in a function too, but since this isn't asking for output, we'll leave it out as its not on topic. Quote
jcvenom Posted July 25, 2013 Posted July 25, 2013 Say what now? I've just gave a perfect example of how to input data. Htmlspecialchars for database input? Are you kidding me? Mysql_real_escape_string() is all that's required since it only escapes what it needs to and therefore keeps the data in the rawest possible form. If you really want to, you could add strip tags, but again, takes away from raw data. Output then needs wrapping in a function too, but since this isn't asking for output, we'll leave it out as its not on topic. lol i never mentioned using Htmlspecialchars for database input read my post again Quote
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.