1

I'm sure someone asked this before but I just can't find a post similar.

how necessary is it to validate an ID field from $_GET variable? I'm using is_numeric() to make sure I'm getting a number at least but am I just putting in unnecessary code?

ex. 

www.test.com/user.php?user_id=5

if (isset($_GET['user_id']) && is_numeric($_GET['user_id'])) {
  *PDO query for user information*
}

is the is_numeric() necessary?
is there a possibility of an attack by changing user_id in the address?

Soujirou
  • 109
  • 6
  • Did you actually try that? `is_int($_GET['user_id'])` will always be `false`, because it's a string. – Ja͢ck May 08 '13 at 02:07
  • possible duplicate of [Typecasting numeric ID to integer to prevent SQL-injection](http://stackoverflow.com/q/15620207), [Properly escaping fields and query settings when using PDO](http://stackoverflow.com/q/10335069), [How does PDO MySQL handle parameters in prepared statements?](http://stackoverflow.com/q/12878182), – mario May 08 '13 at 02:09
  • actually sorry I was using is_numeric() – Soujirou May 08 '13 at 02:18
  • it's not just to prevent against SQL injection, I'm already using PDO for my queries. I wanted to know if there is any other security issues that I would need to check if the ID is actually an ID – Soujirou May 08 '13 at 02:21

5 Answers5

2

The best way to sanitize a numeric id is by using an (int) cast.

$id = (int) $_GET['ID'];

with strings you just never know.

Is the is_int() necessary?

You are probably looking for retrieving data by id. Therefore convert the string to an int is the simplest way to go. On a side note is_int will always return false if applied to a string.

Is there a possibility of an attack by changing user_id in the address?

Well, strings are always dirty. You never know what strange characters an user might input and how that will effect the query. For example, I don't know if it can be applied in this case but, you should take a look at NULL bytes attacks.

Shoe
  • 74,840
  • 36
  • 166
  • 272
  • I notice (int)$_GET['id'] will not pass anything if $_GET['id'] don't exist. if (isset($_GET['id']) && (int)$_GET['id']) – Soujirou May 08 '13 at 02:14
  • @Soujirou, what do you mean by *pass*? – Shoe May 08 '13 at 02:15
  • sorry I didn't complete my sentence when I pressed enter. I want to pass a T/F value so if the ID is int then I can do the query. – Soujirou May 08 '13 at 02:17
1

If you don't want to use prepared statements, PDO::quote should be the correct function:

Returns a quoted string that is theoretically safe to pass into an SQL statement.

Shoe
  • 74,840
  • 36
  • 166
  • 272
WaPaRtY
  • 500
  • 3
  • 5
1

If you want to properly validate an integer before performing the query, you should use filter_input(); the outcome is either a valid integer, false if it's not a valid integer or null if the parameter wasn't passed at all.

if (is_int($userId = filter_input(INPUT_GET, 'user_id', FILTER_VALIDATE_INT))) {
  *PDO query for user information*
}

If you're using prepared statements this won't really matter so much, but if you wish to return a failure response based on whether the input conforms to what's expected, you can use the above.

Ja͢ck
  • 170,779
  • 38
  • 263
  • 309
0

is_int will not work, because GET variables are always passed as strings.

Personally, I like to test for a valid integer with:

if(strval(intval($_GET['user_id'])) === $_GET['user_id'])

However, this can be overkill. After all, if you're using prepared statements then there's no need to handle any escaping, and searching for a row that doesn't exist will just return no results. I'd throw in intval($_GET['user_id']), but only to really make it clear to future coders that the ID is a number.

Niet the Dark Absol
  • 320,036
  • 81
  • 464
  • 592
  • There's some merit in rejecting bad requests. Btw, that integer test just hurts me eyes ;-) – Ja͢ck May 08 '13 at 02:13
0

is_int check type of variable. But $_GET['id'] will be always a string. Better use filter_var.

But you must use prepared statement anyway.

P.S. With prepared statements you can not use validation. DB will tell that nothing was found. But if you want to warn user about bad request you must validate it before querying.

sectus
  • 15,605
  • 5
  • 55
  • 97
  • wouldn't the code be shorter just to check if the ID is an int? I just want to make sure there is no tampering with the link – Soujirou May 08 '13 at 02:27
  • @Soujirou, read the first comment about is_int. About "tampering"... you can't be sure about this. If even id would be valid number. What you want to do if ID would be a valid number? – sectus May 08 '13 at 02:32