CODEWITHSUNDEEP
phpsecurity
Basic
Basic

Reputation: 1866

Do I need to use mysql_real_escape_string on all form inputs?

I know I need to use them on user input fields such as a username entry field, but what about for radio buttons such as a gender option?

Upvotes: 0

Views: 3130

Answers (8)

Headshota
Headshota

Reputation: 21439

Yes you need it. Users can change form inputs as they prefer. Firebug does miracles.

Upvotes: 1

Marc B
Marc B

Reputation: 360752

A pop quiz:

Given a small HTML form snippet:

<input type="hidden" name="studentID" value="42" />
<input type="radio" name="gender" value="Robert'; DROP TABLE Students" />

and your server-side code:

$studentID = $_POST['studentID'];
$gender = $_POST['gender'];

$sql = "UPDATE Students SET gender='$gender' WHERE studentID=$studentID";
$res = mysql_query($sql);

How many ways is this code wrong?

@Basic, at mininum, here's the 'big' errors:

  1. failure to confirm that the student ID is an integer.
  2. Failing to confirm that the studentID actually is valid and exists in the DB.
  3. inserting the studentID directly into the query, potentially allowing a direct SQL injection attack.
  4. Round-tripping a critical value (studentID) through a client-side form, allowing the user to hack the form and change the gender of any student.
  5. Failure to validate the gender value, allowing arbitrary data into the database (e.g. little Bobby Tables is now of gender 'n/a')
  6. inserting the gender value directly into the query string, leading to a direct SQL injection attack (in this case, the Students table gets deleted).
  7. Failure to check the return value from mysql_query(). Queries can be syntactically perfect, yet still fail for other reasons. Never forget to check a query call's status after it completes.

Not so big a deal when only the gender of a student is at issue. Maybe a form letter goes out about "Ms. Robert" and "her" latest detention.

Very big deal if it's an online banking site and the SQL injection just transferred your chequeing account's balance to some offshore account in the Cayman Islands.

Upvotes: 2

Charles
Charles

Reputation: 51411

Stop!

You seem to be confusing escaping with data validation and data sanitization.

You need to validate any data that comes in. Yes, this means making sure that radio buttons contain legal values.

You need to sanitize any data that comes in. Should that text field contain HTML? No? strip_tags. Should that field be a number? Cast it as an integer.

You need to escape any data that you place in the database. If you're still using the prehistoric "mysql" extension, this means using mysql_real_escape_string on everything as you build your query -- not before.

You need to escape any data you echo to the user. htmlspecialchars is your friend.

I've previously explained this in more detail, though this is not a duplicate question.

Upvotes: 5

Paul Dragoonis
Paul Dragoonis

Reputation: 2333

A nice way that you could secure your inputs from $_POST is by running array_map() this is much quicker than a manual foreach().

$formData = array_map('mysql_real_escape_string', $_POST);

You can even encapsulate the process of this by making a getPost() function.

function getPost($key) {
    return array_key_exists($key, $_POST) 
        ? mysql_real_escape_string($_POST[$key]) 
        : null);
}

Hope this helps.

Upvotes: 0

Jonah
Jonah

Reputation: 10091

Yes. Actually, you should also be checking the value of radio buttons/lists to make sure that the value is a valid one. See in_array(). Here's an example (isset check omitted for clarity):

$validValues = array('gold', 'silver', 'bronze');

$value = $_POST['value'];

if (in_array($value, $validValues)) {
    // request is valid
} else {
    // request is invalid
}

The only time you shouldn't use mysql_real_escape_string(), and that's on values that you expect to be integers. But you must handle them carefully. Read this article:

http://www.webappsec.org/projects/articles/091007.shtml

That article covers situations in which you wouldn't expect SQL injection, including how to handle integers.

Upvotes: 0

Michael Krelin - hacker
Michael Krelin - hacker

Reputation: 143189

Well, you should do that to all strings that you pass to mysql, not necessarily network input ;-)

Upvotes: 1

Teneff
Teneff

Reputation: 32158

There are many ways (for the user) to change the submitted data, so - yes you need to use it for radio button.

For example you can change a radio button into an input type text and easily modify the value.

Upvotes: 1

Patrick Evans
Patrick Evans

Reputation: 42736

ALL fields that are sent from a form to a server should be checked for valid inputs. All fields can be manipulated in such a way to do sql or other injection.

Upvotes: 2

Related Questions