 |
|
|
| View previous topic :: View next topic |
| Author |
Message |
pixelterra
Joined: 29 Feb 2008 Posts: 9
|
Posted: Thu Jun 05, 2008 12:17 am Post subject: DB_TABLE sql injection concerns |
|
|
There's something I haven't been able to determine regarding SQL injection protection with DB_TABLE. Since this is a security issue, I have to be DEAD sure I know what's going on before I recommend that db_table be used for an upcoming project.
I know that I can preview the sql statement built with sql query arrays using buildSQL(), but how do I see the sql statement that will be used when using insert/update/delete statements before they are exectuted.
It is not clear from the documentation if I need to manually escape the data values for insert/update/delete or if this is taken care of automatically with db_table and I can't find a way to look at the actual sql generated behind the scenes. Since there is no DB_Table_Database->escape() function, I've been using the $mdb2->escape() func to make sure my "where" values for upadate statements are safe.
The following code might highlight my confusion:
$row = array(
'first_name' => "ba'd",
);
$where = "' OR 'a' = 'a";
$r = $db->update('test', $row, "username = '$where'");
The the value in the data array seems to be automatically escaped, but the value of $where isn't, but the value of the 'where' argument (#3) isn't, and thus the above is a valid SQL injection. I can solve this problem like this:
$where = $mdb2->escape($where);
But I'm still confused as to whether this should be necessary or not. When I manually escape the data in the data array, the escapes appear in the db:
$row = array(
'first_name' => $mdb2->escape("ba'd"),
);
So it seems the data is automatically escaped, but not the 'where' arguement. So my questions are these: s it a valid conclusion that the values in the data array are automagically escaped, but values used in other places of the sql statement are not, like the $where clause above?
BML |
|
| Back to top |
|
 |
mark

Joined: 07 Jan 2007 Posts: 1013
|
Posted: Thu Jun 05, 2008 12:15 pm Post subject: Re: DB_TABLE sql injection concerns |
|
|
| pixelterra wrote: | | There's something I haven't been able to determine regarding SQL injection protection with DB_TABLE. Since this is a security issue, I have to be DEAD sure I know what's going on before I recommend that db_table be used for an upcoming project. |
I have good news for you. See below:
| pixelterra wrote: | | I know that I can preview the sql statement built with sql query arrays using buildSQL(), but how do I see the sql statement that will be used when using insert/update/delete statements before they are exectuted. |
delete() uses a "normal" SQL query with your $where parameter. insert() and update() use the autoExecute() method of DB and MDB2. These query can be viewed after execution via the $last_query property (at least in DB, MDB2 should have something similar).
| pixelterra wrote: | | It is not clear from the documentation if I need to manually escape the data values for insert/update/delete or if this is taken care of automatically with db_table and I can't find a way to look at the actual sql generated behind the scenes. Since there is no DB_Table_Database->escape() function, I've been using the $mdb2->escape() func to make sure my "where" values for upadate statements are safe. |
Escaping/quoting is your job for the "where" arguments, but not for the values that you want to insert or update.
For example, if you want to delete a record, it is recommended to it like this:
| Code: | | $object->delete('id = ' . $object->quote($id)); |
This quote() method is a simple wrapper around quoteSmart() (DB) and quote() (MDB2).
If you want to insert values, it is safe to pass just your value array to insert(). (Of course, it makes sense to validate the values before inserting them, but SQL injection isn't a problem even without validation.)
If you want to update a record, the value array is also safe, but you have to take care of the the $where parameter, e.g:
| Code: | | $object->update($value_arr, 'id = ' . $object->quote($id)); |
Finally, there is a special case for select() calls. In general, you'll have to take care of the $where parameter just as with delete() and update(), e.g.:
| Code: | | $record = $object->select('one', 'id = ' . $object->quote($id)); |
But there is also an auto-quoting feature available (using DB's and MDB2's features):
| Code: | | $record = $object->select('one', 'id = ?', null, null, null, array($id)); |
| pixelterra wrote: | | So it seems the data is automatically escaped, but not the 'where' arguement. So my questions are these: s it a valid conclusion that the values in the data array are automagically escaped, but values used in other places of the sql statement are not, like the $where clause above? |
Yes, exactly. The reason is that values from the arrays can easily be quoted, while the $where parameter can basically be anything. And auto-quoting isn't possible for "anything".
If you have more questions, don't hesitate to ask. And if you have ideas on how this topic could be better covered in the manual, please tell me (here, via email, via the bug tracker, ...).
Your two emails had the same content as your post here, so I won't send you the same answer via email. New emails will get an answer, of course.  |
|
| Back to top |
|
 |
pixelterra
Joined: 29 Feb 2008 Posts: 9
|
Posted: Thu Jun 05, 2008 6:12 pm Post subject: |
|
|
| Pixelterra wrote: |
I know that I can preview the sql statement built with sql query arrays using buildSQL(), but how do I see the sql statement that will be used when using insert/update/delete statements before they are exectuted. |
| Mark wrote: |
delete() uses a "normal" SQL query with your $where parameter. insert() and update() use the autoExecute() method of DB and MDB2. These query can be viewed after execution via the $last_query property (at least in DB, MDB2 should have something similar). |
Thank you for your thorough and quick response. You've answered almost all of my questions except one. My question below was how to view the prepared query BEFORE it is executed. This is crucial, at least to me. I found myself making invalid sql just to view the error object. Unfortunately, the query part of that object isn't the actual SQL, as the where clause has a ? in it, and other values are quoted in a confusing way.
Is there any way to see the qry BEFORE executing it? I feel as if I'm being forced to cross a street blind-folded.
If there is currently no way, perhaps an extension to the buildSQL method would be useful:
$db->buildSQL('update', $table, $data, $where);
$db->buildSQL('insert', $table, $data);
On Injections:
Of course though your assurance about quoting/escaping has made me more confident, without this assurance, it isn't clear from the documentation what is going on. I'd recommend making this issue an explicit part of the documentation.
Thanks again.
Ben |
|
| Back to top |
|
 |
mark

Joined: 07 Jan 2007 Posts: 1013
|
Posted: Thu Jun 05, 2008 6:24 pm Post subject: |
|
|
| pixelterra wrote: | | Thank you for your thorough and quick response. You've answered almost all of my questions except one. My question below was how to view the prepared query BEFORE it is executed. This is crucial, at least to me. I found myself making invalid sql just to view the error object. Unfortunately, the query part of that object isn't the actual SQL, as the where clause has a ? in it, and other values are quoted in a confusing way. |
DB_Table uses the autoExecute() method of DB or MDB2, and therefore, DB_Table can't provide the SQL query that is being generated. I understand your concerns about security, but why isn't it enough for you to see the generated SQL query after execution?
I'm not sure about "other values are quoted in a confusing way". Can you please elaborate on this?
| pixelterra wrote: | | If there is currently no way, perhaps an extension to the buildSQL method would be useful:[...] |
I see your point, but providing such a feature would basically require to re-implement hundreds of lines of code from DB and MDB2 (including specific code for the supported RDBMS drivers).
| pixelterra wrote: | | Of course though your assurance about quoting/escaping has made me more confident, without this assurance, it isn't clear from the documentation what is going on. I'd recommend making this issue an explicit part of the documentation. |
Okay, a short chapter with some examples makes sense, yes. I've added an item to my TODO list. |
|
| Back to top |
|
 |
|
|
You cannot post new topics in this forum You cannot reply to topics in this forum You cannot edit your posts in this forum You cannot delete your posts in this forum You cannot vote in polls in this forum
|
|