Subject: | This class isn't secured at all and... |
Summary: | Package rating comment |
Messages: | 4 |
Author: | Artur Graniszewski |
Date: | 2011-03-07 12:50:13 |
Update: | 2011-03-08 11:40:26 |
|
|
|
Artur Graniszewski rated this package as follows:
Utility: | Insufficient |
Consistency: | Good |
Examples: | Good |
|
 Artur Graniszewski - 2011-03-07 12:50:13
This class isn't secured at all and is very vulnerable on SQL injections.
First and worst of all mysql_escape_string() does not escape backtick (`) character at all - so the attacker can easily write entire SQL query as your table name like so:
` WHERE 1=1 --
The second problem is that you should use mysql_real_escape_string() because some power users use two or more different connections to different MySQL databases (probably with different charset encodings), so there can be a mess, because mysql_escape_string uses only the first connection it founds.
 Artur Graniszewski - 2011-03-07 12:55:00 - In reply to message 1 from Artur Graniszewski
I've made a mistake in the SQL injection sample, the working one is:
tablename` WHERE 1=1 --
 Garry Lachman - 2011-03-08 11:21:48 - In reply to message 2 from Artur Graniszewski
thanks, fixed...
 Artur Graniszewski - 2011-03-08 11:40:27 - In reply to message 3 from Garry Lachman
This is not good. SQL injections can still be executed the same way I described above.
Looking at your code I see wrong changes to:
$query = sprintf("SELECT * FROM `%s`",mysql_real_escape_string($table));
First of all: mysql_real_escape_string() function should use MySQL connection identifier (resource) as it's second argument
Second: mysql_real_escape_string() is similar to mysql_escape_string() and does not escape backtick character. So my example SQL injection works fine on your class. Every other security measures you implemented are negated by this one security hole.
Also, the same applies to the column names (there is another security hole using unescaped backtick character).
|