5

I use is_numeric for everything, the only input I get from users is a form with an Student ID number....

I recently was reading up on SQL Injections and was wondering if the following precaution is necessary?

Currently I have:

if(is_numeric($_POST['sid']){
     $sid = $_POST['sid']; 
     $query = "select * from student where sid='".$sid."'";
     // More Code...
}

What I've read is safer

if(is_numeric($_POST['sid']){
     $sid = (int) $_POST['sid'];
     $query = "select * from student where sid='".$sid."'";
     // More Code...
}

Is one version really safer than the other? How would someone bypass 'is_numeric'?

Also, would this be any less safe than what I currently have?

if(is_numeric($_POST['sid']){
     $query = "select * from student where sid='".$_POST['sid']."'";
     // More Code...
}

So, I guess what I am really asking, is if one of these code blocks is truly safer than another one


EDIT: Sorry I didn't state this early but I am using oracle 10g database, not mysql. With oci_connect();

Arian Faurtosh
  • 17,987
  • 21
  • 77
  • 115

3 Answers3

9

Why are you going through these questions asking about how you can sanitize your inputs so that you can build SQL statements from outside data? Building SQL statements from outside data is dangerous.

Rather than wasting your time worrying about how you can come up with another halfway thought out "solution", stop and put on your Big Programmer Pants and start using prepared statements and bound variables.

Here is a fantastic answer that will get you started: How can I prevent SQL injection in PHP?

You can also check http://bobby-tables.com/php for other examples.

Looks to me like you can still do prepared statements and bound variables with Oracle: http://php.net/manual/en/function.oci-bind-by-name.php or through PDO http://php.net/manual/en/pdostatement.bindparam.php

Community
  • 1
  • 1
Andy Lester
  • 91,102
  • 13
  • 100
  • 152
  • 1
    "Big Programmer Pants" >_ – user2246674 Sep 05 '13 at 21:16
  • I am not using a mysql database, I am using and oracle database 10g using oci_connect. – Arian Faurtosh Sep 05 '13 at 21:17
  • 1
    @Arian: Looks to me like you can still do prepared statements and bound variables with Oracle: http://www.php.net/manual/en/function.oci-bind-by-name.php or through PDO http://php.net/manual/en/pdostatement.bindparam.php – Andy Lester Sep 05 '13 at 21:20
  • @AndyLester Can you add that into your answer... So I can mark it as answered? – Arian Faurtosh Sep 05 '13 at 21:25
  • Any database driver implementation worth its salt has support for bound variables and prepared statements. If you're using a driver implementation that doesn't, then get rid of it. Immediately. –  Sep 05 '13 at 21:28
  • What is a wearer of big programming pants to do when they need to be able to programmatically modify parts of the statement depending on how variables are set, which seems to preclude the use of prepared statements. For example, if the $_POST['gender'] variable isset then I want to add it to my where clause, if it isn't set I want to leave it off of the where clause entirely. – Corey Feb 26 '14 at 05:39
  • Then modify the SQL to add part of the WHERE clause, using a placeholder of course, and also add $_POST['gender'] to your bind variables array. – Andy Lester Feb 26 '14 at 17:13
  • This is all good information and I agree with the sentiment but it is _not_ an answer to the actual question. There may be many good reasons to need to know the answer, including working in legacy code where restructuring for a prepared statement is too big a change. – Samuel Åslund Nov 05 '18 at 09:56
8

As a rule of thumb, it is not safe enough to just check the data, you should always preprocess the data before using it.

Let’s say you don’t know all the internals of how the is_numeric works. Or you don’t know if its logic or its implementation will change in future. Or you did not test it heavily enough. Or its documentation lacks some very specific but important note.

All in all, one day it just might happen that is_numeric returned TRUE but the data is not safe to be used in SQL.

However, if you did not stop after checking input data, but instead created new piece of data based on input, you can definitely say what this data of yours is and what it isn’t. In this case, it is definitely an integer value and nothing else. (While is_numeric also works for float values, did you know that? Did you intend that? No?)

And just another example: if your input is a string in hexadecimal, octal or binary notation, is_numeric will return TRUE, but your database might not even support this particular notation (MySQL does not support octal, for instance), so you will not get expected results.

Denys Popov
  • 1,040
  • 12
  • 14
5

I would not endorse either version. What you should be using is properly parameterized queries with PDO or mysqli. For example:

$query = "SELECT * FROM student WHERE sid = ?";
$stmt = $pdo->prepare($query);
$stmt->execute(array($_POST['sid']));

In this case the vulnerability is handled at the database access level and not at the application level. It may not seem like a big deal, but someone could move the query outside of the if statement and improperly use the query (you could make the argument that someone could also rewrite a vulnerable query, but that would be less your fault).

You can also bind values with types.

$stmt = $pdo->prepare($query);
$stmt->bindValue(1, (int)$_POST['sid']);
Explosion Pills
  • 188,624
  • 52
  • 326
  • 405