Is it safe to only use intval to sanitize user input for a database select?

Question

Is it safe to only use intval to sanitize the user input for a database select, as in the following example?

$id = intval($_GET['id']);

$query = "SELECT * FROM table WHERE id='$id'";

Answer 1

intval is guaranteed to produce an integer (it returns 0 if it "fails"), and an integer is a safe thing to use in a MySQL query. This answer https://stackoverflow.com/a/3208089/458614 on SO agrees:

Yes, intval() is safe. There is absolutely no way to perform an SQL injection when the parameter is converted to integer, because (obviously) the format of an integer does not allow putting SQL keywords (or quotes, or whatever) in it.

I suspect the same goes for uses other than SQL.

Answer 2

Is it safe to only use intval to sanitize the user input for a database select, as in the following example?

Short answer: No.

OK, intval() will always deliver you an integer value which will not break your SQL.

The problem is that, whilst the value returned is an integer, is it a valid one? Can you guarantee that the value returned by intval (e.g. 0) will never correspond to a real data record? With code that does no further checking, you could inadvertently create a record with a key of 0 and then have many users access the same record, all because they're [all] entering invalid numeric values. Not good.

All for the want of a polite error message like "ID must be numeric".

Answer 3

Rule of thumb is: If the input comes from anywhere else than from the code itself, the data provided is to be considered 'tainted'. You will have to make sure the value is what you expect and would not mess up your SQL statement (injection).

Though I question why you're not using/utilizing PDO and use markers in your statement so these things are handled automatically.

But I would choose to do something like this (using a ternary operator):

$id = (is_numeric($id) && $id > 0) ? $id : false;

if ($id !== false) {
   ... code goes here ...
}

It doubles the possibility of $id being a realistic integer.

Answer 4

First, check if $id is a valid numeric value ( using is_numeric ) then finally cast the $id to type ( int ).

Answer 5

Ints are by definition safe for any situation I can think of, though for use as a param to a CLI command, ensure they aren't negative as the leading hyphen may mean they become taken as flags instead of params; and for cases where there's a storage size limits, beware of bounds as overflows may be unfortunate.

However, I'd be inclined to create a bunch of user-input validation methods, if you are doing this often. Something like:

public function getValidInt(
    string $name,
    int $minValue=PHP_INT_MIN,
    int $maxValue=PHP_INT_MAX,
    int $defaultValue=0
) {
    assert($minValue < $maxValue, 'Min bound must be less than max.');
    if (!array_key_exists($name, $_GET)) {
        return $defaultValue;
    }
    $userValue = (int)$_GET[$name];
    // Could instead return $defaultValue if it's out of bounds.
    return min($maxValue, max($minValue, $userValue));
}

This makes it super-easy to get yourself untainted, bounds-checked values with a valid default value if they are not specified.

For strings, this might look like:

public function getValidString(
    string $name,
    string $validationRegex='.*',
    int $minLength=0,
    int $maxLength=PHP_INT_MAX,
    int $defaultValue=''
)

...and so forth.

Adding such things early on in your project means you'll be thinking about every user input, and will not amass a huge tech debt task of "validate all user input" as you near the end of the project. Everything will already be correct; strings will be whitelisted, ints will be bounds-checked, and so forth.

Escaping and detainting on input should ideally be treated as a separate concern from escaping for use in a database, display as HTML, etc. Removing malicious HTML tags from strings is something you should be doing on input (there should be nobody called <span onclick="alert('whee')">Fred</span> stored in your database, even if you have not yet displayed their name to the screen. Escaping before you throw a variable to the HTML or MySQL or commandline should be your final last-chance confirmation that you've ensured save variables, not be your only check.