-2

Possible Duplicate:
How to prevent SQL injection in PHP?

I have a following MySQL query :

if($obj->{'parentId'} == null){
    $parentID = 'NULL'; 
} else{
    $parentID = $obj->{'parentId'};
}

$q = 'UPDATE tasks SET 
    Name = "'.$obj->{'Name'}.'", 
    Cls = "'.$obj->{'Cls'}.'", 
    parentId = '.$parentID.', 
    PhantomId = '.$obj->{'PhantomId'}.', 
    PhantomParentId = '.$obj->{'PhantomParentId'}.', 
    leaf = "'.$leaf.'" WHERE Id = "'.$obj->{'Id'}.'"';

The problem is, that if any of my non-string values is empty, the whole query throws error. How can I fix it crashing when for example $obj->{'PhantomId'} is empty without any aditional libs ?

Community
  • 1
  • 1
mike_hornbeck
  • 1,612
  • 3
  • 30
  • 51
  • 5
    **tldr; use bound parameters** see http://stackoverflow.com/questions/60174/how-to-prevent-sql-injection-in-php –  Jan 15 '13 at 02:07
  • The answer is too easy: Make just sure that they are not empty! :) And if so, set proper defaults – hek2mgl Jan 15 '13 at 02:09
  • @hek2mgl That's an awful answer. What if $obj->{`Name`} is not-empty but is a value that otherwise causes invalid syntax or allows an exploit? –  Jan 15 '13 at 02:11
  • That was not the question. If you like to point to questioner to prepared statements, please do so. But when I read the question I have to assume he should learn about the `if` statement first ;) – hek2mgl Jan 15 '13 at 02:13
  • @pst is mysqli/pdo included in php5.3 ? – mike_hornbeck Jan 15 '13 at 02:14
  • @hek2mgl I was looking for something that won't require thousand ifs :) – mike_hornbeck Jan 15 '13 at 02:15
  • And I'm not a beginner really, but I was using django's Orm for few years now without bare sql statements – mike_hornbeck Jan 15 '13 at 02:15
  • @mike You'll need those ifs. Of course. what do you expect the 'library' should do for you? – hek2mgl Jan 15 '13 at 02:16
  • I was hoping for some escapes or whatever, not really sure. This looks to be really a vulnerable construction. – mike_hornbeck Jan 15 '13 at 02:17
  • @hek2mgl See, that is wrong. He *may* need *some* conditionals, depending upon values. –  Jan 15 '13 at 02:17
  • 2
    @mike_hornbeck It is very insecure - fix to use prepared statements first and this problem will be *much* easier to deal with. See the link posted. Both `mysqli` and `pdo` have been around *for ages*. You will still need conditionals to alter the query to *not set* a value (which is different than assigning `NULL` or another suitable default), but if values are to be assigned in all cases then there are a number of simple coalescing techniques - and in the case of null->`NULL`, usually *nothing has to be done*! –  Jan 15 '13 at 02:18
  • 1
    @mike_hornbeck You may be interested in the [ternary operator](http://php.net/manual/en/language.operators.comparison.php) to help with coalescing (there are other patterns that utilize other conversion rules). Remember to use strict equality unless careful examination of the true/false input types shows the "true value" form is appropriate. –  Jan 15 '13 at 02:23

1 Answers1

3

Better consider to opt out to bound parameters. But if you still want to construct SQL queries use conditions

$q = "UPDATE ...";
...
if (!empty($obj->{'PhantomId'})) {
    $q .= ", PhantomId = '" . $obj->{'PhantomId'}. "'";
}
...
peterm
  • 91,357
  • 15
  • 148
  • 157
  • ... or use quotes around the values. – uınbɐɥs Jan 15 '13 at 02:22
  • (to be more clear) You *can avoid* conditions if you just put quotes around the values (but of course it is better to use prepared statements with PDO / MySQLi). – uınbɐɥs Jan 15 '13 at 02:34
  • In some cases yes, but putting quotes is not the same as not using the column in an UPDATE statement at all or conditionally setting necessary value (NULL or any predefined value) if the property value is empty. It totally depends on the app logic. – peterm Jan 15 '13 at 02:47