Package home | Report new bug | New search | Development Roadmap Status: Open | Feedback | All | Closed Since Version 1.6.4

Bug #8407 Database containers don't quote table/field names
Submitted: 2006-08-10 03:43 UTC
From: pear at adamharvey dot name Assigned: aashley
Status: Closed Package: Auth (version CVS)
PHP Version: Irrelevant OS: Irrelevant
Roadmaps: (Not assigned)    
Subscription  


 [2006-08-10 03:43 UTC] pear at adamharvey dot name (Adam Harvey)
Description: ------------ The four database containers that exist in Auth (DB, DBLite, MDB and MDB2) currently use the table and field names as is without any quoting. Furthermore, the documentation at present doesn't actually inform the developer that the table and field names should be quoted. The linked patch quotes identifiers before using them in SQL queries, thereby avoiding any possibility of SQL injection. (Any injection attack would still require some seriously sloppy code on the part of developer using Auth, but this closes the door fairly decisively.) This patch *will* break BC for the case where Auth users are already quoting identifiers before passing them to the container. The behaviour of the db_fields option has also changed somewhat, as the case where a string ", field_a, field_b" was given for this option will now break. Note that it wouldn't have worked without the preceding comma anyway; the preferred way of doing this has always been array('field_a', 'field_b') to my mind. In short, this is probably a candidate for Auth 2 rather than 1.x. Test script: --------------- Patch is at: http://www.adamharvey.name/patches/Auth-container-quoting.patch

Comments

 [2006-08-11 01:15 UTC] aashley at php dot net (Adam Ashley)
Patch merged to CVS, rolling RC to get feedback from users.
 [2006-08-16 23:23 UTC] ohm at morishima dot net (Akitoshi MORISHIMA)
The simplest example below, just like in the Introduction - Auth tutorial in the documentation: <?php require_once "Auth.php"; $dsn = "pgsql://user@localhost/database"; $a = new Auth("DB", $dsn); $a->start(); if ($a->checkAuth()) { echo "login completed.\n"; } else { echo "login incorrect.\n"; } ?> does not work. PostgreSQL complains that ERROR: zero-length delimited identifier at or near """" at character 32 and the SQL in question was: SELECT "username", "password", "" FROM "auth" WHERE "username" = 'testuser' I didn't test on MySQL but it's likely similar error will be produced. This is because the default value of options['db_fields'] is '' (that is, not 'unset') and the patched _quoteDBFields() just checks the value is set or not, and quoteIdentifier($str) (DB/common.php) does not check $str is empty string or not. Quick fix, add 2 lines to _quoteDBFields(): function _quoteDBFields() { if (isset($this->options['db_fields'])) { if (is_array($this->options['db_fields'])) { $fields = array(); foreach ($this->options['db_fields'] as $field) { if (!$field) continue; // <- added $fields[] = $this->db->quoteIdentifier($field); } return implode(', ', $fields); } else { if (!$this->options['db_fields']) return ''; // <- added return $this->db->quoteIdentifier($this->options['db_fields']); } } return ''; }
 [2006-08-17 01:04 UTC] aashley at php dot net (Adam Ashley)
Thanx for catching that Akitoshi, fix in CVS.
 [2006-08-21 02:20 UTC] aashley at php dot net (Adam Ashley)
Thank you for your bug report. This issue has been fixed in the latest released version of the package, which you can download at http://pear.php.net/get/Auth