Thread: [Postfixadmin-devel] Using SQL prepared statements -> getting rid of mysql extension
Brought to you by:
christian_boltz,
gingerdog
From: Jan R. <ja...@ro...> - 2009-01-25 16:30:53
|
Hello List, I mentioned in my participation request a few days ago that I think there are some theoretical SQL injection vulnerabilities in postfixadmin. Have a look at the login process: $result = db_query ("SELECT password FROM $table_admin WHERE username='$fUsername' AND active='1'"); if ($result['rows'] == 1) { $row = db_array ($result['result']); $password = pacrypt ($fPassword, $row['password']); $result = db_query ("SELECT * FROM $table_admin WHERE username='$fUsername' AND password='$password' AND active='1'"); if ($result['rows'] != 1) { $error = 1; $tMessage = $PALANG['pLogin_failed']; $tUsername = $fUsername; } } I think the main problem is the usage of "unprepared" sql statements. Somebody may form a username like "' OR true" ore something like that and the sql statement ends up in a completely different meaning. This is a very theoretical vulnerability as there is a second query after the password is hashed. But can't we get rid of the mysql extension compatibility and force usage of mysqli (ore pgsql) to make use of prepared statements? With prepared statements we can render sql selects like "SELECT * FROM $table_admin WHERE username=? AND password=? AND active='1'", which is much more secure than the method mentioned above. Postgres supports prepared statements this since version 7.4 which was released in 2004 I think. I would like to do this - maybe in a svn branch. I'm looking forward to your comments. Best regards Jan -- Jan Röhrich Kleinglattbacher Str. 12 D-75428 Illingen Tel.: 07042 120351 Mobil: 01638463295 eMail: ja...@ro... |
From: David G. <da...@co...> - 2009-01-25 20:09:51
|
Jan Röhrich wrote : > Hello List, > > I mentioned in my participation request a few days ago that I think > there are some theoretical SQL injection vulnerabilities in postfixadmin. > > Have a look at the login process: > > > $result = db_query ("SELECT password FROM $table_admin WHERE > username='$fUsername' AND active='1'"); > if ($result['rows'] == 1) > { > $row = db_array ($result['result']); > $password = pacrypt ($fPassword, $row['password']); > $result = db_query ("SELECT * FROM $table_admin WHERE > username='$fUsername' AND password='$password' AND active='1'"); > if ($result['rows'] != 1) > { > $error = 1; > $tMessage = $PALANG['pLogin_failed']; > $tUsername = $fUsername; > } > } > > I think the main problem is the usage of "unprepared" sql statements. > Somebody may form a username like "' OR true" ore something like that > and the sql statement ends up in a completely different meaning. This is > a very theoretical vulnerability as there is a second query after the > password is hashed. > > But can't we get rid of the mysql extension compatibility and force > usage of mysqli (ore pgsql) to make use of prepared statements? With > prepared statements we can render sql selects like "SELECT * FROM > $table_admin WHERE username=? AND password=? AND active='1'", which is > much more secure than the method mentioned above. Postgres supports > prepared statements this since version 7.4 which was released in 2004 I > think. > > I would like to do this - maybe in a svn branch. I'm looking forward to > your comments. Hi! We've kind of had this debate in the past...... a) Someone will complain about using $library (which will be needed to emulate prepared statements for MySQL) b) All fields should have 'escape_string' run on them before being used... so 'should' be safe. Obviously this requires $developer remembers to do whatever. c) It's quite likely that we'll migrate to using Zend_Db_Table/Zend_Db (which implies PDO) soon ... so although it's a good idea for you to patch any bugs in the current codebase, it's not worth porting all the code to use a library which provides agnostic prepared statements as your work will be in vain if we do move to Zend_Db.... thanks David. -- David Goodwin [ david at codepoets dot co dot uk ] [ http://www.codepoets.co.uk ] |