|
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 ]
|
