views:

555

answers:

3

I've never liked wrapping the

mysql_real_escape_string

function around input I expect to be integer for inclusion in a mysql query. Recently I came across the

filter_var

function. Nice!

I'm currently using the code:

if (isset($idUserIN) 
    && filter_var($idUserIN, FILTER_VALIDATE_INT) 
    && 0 < filter_var($idUserIN, FILTER_SANITIZE_NUMBER_INT)
    ) {
      $idUser = filter_var($idUserIN, FILTER_SANITIZE_NUMBER_INT);
      $sql = 'SELECT * FROM TABLE_NAME WHERE idUser = '.$idUser;
} else {
  // handle invalid data
}

Does this leave any holes open?

('> 0' chosen rather than '>= 0' as its a table auto_increment field, so 0 would not be a normal value)

+1  A: 

A much simpler and easier-to-read method that I use is this:

$sql = 'SELECT * FROM TABLE_NAME WHERE idUser = ' . intval($idUser);

It attempts to convert $idUser to an integer and on failure returns 0 which none of my tables have as real id's. (So I know the input was invalid if it evaluates to 0.)

To answer your actual question, no that won't leave any holes open. I suggest getting rid of the repetitive code though:

$idUserIN_filtered = filter_var($idUserIN, FILTER_VALIDATE_INT);

if (isset($idUserIN) 
    && $idUserIN_filtered 
    && 0 < $idUserIN_filtered
    ) {
      $sql = 'SELECT * FROM TABLE_NAME WHERE idUser = '.$idUser_filtered;
} else {
  // handle invalid data
}
yjerem
+2  A: 

I myself would create a function for that task, possibly in a static class somewhere,

  public static function escape_int($i)
  {
     $sanitised = intval($i); 
     if( '_' . $sanitised . '_' === '_' . $i . '_'  && $sanitised > 0 ) 
     {
        return $sanitised;
     }
     throw new IntegerEscapeException( $i, $sanitised );
     return "ENOINT"; # Wont Run This, but I prepare for the impossible. 
  }

try 
{ 
   $sql = 'SELECT * FROM TABLE_NAME WHERE idUser = ' . DB::escape_int( $userid ); 
   DB::query($sql); 
   ...etc...
}
catch( IntegerEscapeException $e )
{ 
   die ( "You shot the sherif!" ); # bad example.
}

This is good because if I discover my sanitation method reeks I can fix it later.

Kent Fredric
A: 

This is all you need

if ($idUser = filter_var($idUserIN, FILTER_VALIDATE_INT)) {
      $sql = 'SELECT * FROM TABLE_NAME WHERE idUser = '.$idUser;
} else {
  // handle invalid data
}

or

if ($idUser = filter_input(INPUT_POST, 'userId', FILTER_VALIDATE_INT)) {

Alternatively you can check if its invalid in MVC.

if (!$idUser = filter_var($idUserIN, FILTER_VALIDATE_INT)) {
      throw new InputParameterException('UserId');
}
//else its valid
OIS