0

I have a query like this:

$result = $db
-> prepare("SELECT value FROM mytable WHERE id = ?")
-> execute(array($id))
-> fetch(PDO::FETCH_ASSOC);

And I want to use $result['value'] as a parameter for another query (an UPDATE statement). Should I use prepared statement for that UPDATE statement? Or because I've taken it from database, then no need to pass it as prepared statement?

Martin AJ
  • 6,261
  • 8
  • 53
  • 111
  • 8
    Yes, you always should. It's called [second level injection](http://stackoverflow.com/questions/1871774/what-is-second-level-sql-injection). Never assume that any data is safe. Ever. – Andrei Jun 08 '16 at 14:54
  • @Andrew But I'm pretty sure what I'm getting from database is safe .. Why shouldn't be? – Martin AJ Jun 08 '16 at 14:56
  • 4
    "Pretty sure" is one of the reasons. ;) – Jonnix Jun 08 '16 at 14:56
  • 1
    @Stack Guilty until proven innocent. – Andrei Jun 08 '16 at 14:56
  • The data can be safe, but a quote in valid data can still break your sql statement. Perhaps not in this specific case where you are probably talking about an integer but in general a string can still contain characters or even sql that breaks or modifies your sql statement if you don't prepare it (or escape the data...). – jeroen Jun 08 '16 at 14:59
  • @Stack Is the data created form user input? There is your reason. Is there anybody but you who has access to the database? There is your reason. Is there any users besides the database admin who an attacker might gain access to and use injection methods to run querys with the rights of a maybe more priviliged user? There is your reason. – Johannes H. Jun 08 '16 at 14:59
  • Alright, I got it .. thank you guys – Martin AJ Jun 08 '16 at 15:01

1 Answers1

0

Yes. Use a prepared statement with a bind placeholder.

Just because a value is being returned from a database doesn't mean that the value is safe for inclusion in SQL text.

You may have domain knowledge that the value column in mytable is INTEGER type, so that it would be safe. But in the more general case, and for the reader who doesn't know the definition of mytable, and what value might contain. A reader of your code is going to assume that value isn't "safe". For all we know, we could be getting something like this:

Robert'); DROP TABLE students; --

Whenever we see a variable concatenated into the SQL text, we are going to assume that the variable could contain something other than a value, and it could contain actual SQL. (Or, if we do see a variable concatenated into the text of a SQL statement, we would be expecting that it's going to be properly escaped right at the point it's being concatenated.)

So, the preferred pattern would be use a prepared statement with a bind placeholder. That makes it unambiguous to the reader that value is indeed a value, and that it's not intended to be interpreted as SQL text.

spencer7593
  • 106,611
  • 15
  • 112
  • 140