1

So using %27 you can just SQL inject even though data is sanitized with mysql_real_escape_string

%27) SQL INJECTION HERE %2F*

What to do?

Edit with example:

$sql = sprintf("SELECT *, MATCH(post) AGAINST ('%s*' IN BOOLEAN MODE) AS score FROM Posts WHERE MATCH(post) AGAINST('%s*' IN BOOLEAN MODE)",
                mysql_real_escape_string($_GET['searchterm']),
                mysql_real_escape_string($_GET['searchterm']));

$results = $db->queryAsArray($sql);

If you pass in %27) SQL INJECTION HERE %2F* to the searchterm querystring, I get outputted on the page:

You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near 'BOOLEAN MODE)' at line 1

Thanks everyone for finding the problem in the db class..

Marcus
  • 9,011
  • 10
  • 45
  • 65

4 Answers4

2

Reasoning from the method name queryAsArray, it seems that you’re using this DbBase class from the comments of the MySQL functions manual page. If so, it’s the query method that removes the escape character from the escaped quotation marks:

function query($sql, &$records = null){
    $sql = str_replace(array('\\"', "\\'"), array('"', "'"), $sql);
    // …
}

Then it’s not a miracle that your example works (I simplified it):

$input = "', BAD SQL INJECTION --";

$sql = "SELECT '".mysql_real_escape_string($input)."'";
var_dump($sql);  // string(33) "SELECT '\', BAD SQL INJECTION --'"
//                      everything’s OK ↑

$sql = str_replace(array('\\"', "\\'"), array('"', "'"), $sql);
var_dump($sql);  // string(32) "SELECT '', BAD SQL INJECTION --'"
//                                Oops! ↑
Gumbo
  • 643,351
  • 109
  • 780
  • 844
1

The note mentioned in our manual has been marked for deletion. Once it propagates across all of the mirrors in our network, it will no longer appear attached to the official documentation.

~ Daniel P. Brown
  Network Infrastructure Manager
  http://php.net/
0

It's best to not to build statements like this at all, and instead use queries with parameters using mysqli or PDO. This will deal with the problem of MySQL injection and one day (not yet, unfortunately) it will perform better too, because the queries are cached without parameters, meaning you only got one query in the cache instead of dozens of different queries because of a single input value changing all the time. Other databases make use of this since long, but MySQL just managed not to make parameterized queries slower since the latest version.

It doesn't look plausible that %27 will actually terminate the string. It seems more like a possibility to embed quotes inside a string, but I'm not sure.

To be sure, I decided to sacrificed my server and test this. When I enter %27 in an input field and textarea that are escaped using mysql_real_escape_string and are then inserted in the database, I get no errors. The text %27 is just inserted. So no problem at all.

GolezTrol
  • 114,394
  • 18
  • 182
  • 210
  • It makes me wonder why everyone who rants about parameterized queries, never brings an example with them. May be it's because the code would be more complex and messy with no real benefits? – Your Common Sense Mar 14 '11 at 22:09
  • he didn't mean literal `%27` but an apostrophe coming from query string. – Your Common Sense Mar 14 '11 at 22:10
  • Sure, but in PHP you got either an apostrof that is escaped by `mysql_real_escape_string` or you got the literal text `%27` which apparently doesn't need to be escaped. So why the downvote? Point is you can't inject SQL this way. The question suggested that an url-escaped quote would pass `mysql_real_escape_string` and would cause trouble in the SQL statement. But it won't unless you're using it in a `LIKE`. :p – GolezTrol Mar 14 '11 at 22:25
  • Coding with parameters is definately not more complex, not messy, although the question-mark parameters are a bit clumsy. I wrote a wrapper object that allows inserting named parameters in the form of `SELECT * FROM table t WHERE t.field = :PARAM`. Then, you can execute this query, passing the SQL and an array in the form of `array('PARAM'=>$value, ...)`. The object takes care of replacing the parameters and preparing the statement. Can't be easier, and you don't get all this escaping-shit thoughout your code with the risk of forgetting it. – GolezTrol Mar 14 '11 at 22:29
  • If you like I'm happy to share this object with you, along with some examples, but since this is one of the first things I wrote in PHP, I can imagine there are better examples to be found. I use it throughout my site hoewever, and it feels safe to know that there's a layer that makes sure I can't ever forget to escape my input, and that makes composing a query easier and cleaner too. – GolezTrol Mar 14 '11 at 22:31
  • I am using similar layer, but based on plain mysql, not prepared statements. So, I see no reason to use native prepared statements. And I am asking an example not for me but for the OP. If you don't supply your answer with useful example, nobody would pay attention on it – Your Common Sense Mar 14 '11 at 22:38
  • I don't see any example in your answer either.... Anyway, here is the object I use. Like I said, it is dirty, but I was only programming PHP for a couple of days then. http://www.goleztrol.nl/demo/SO/PHP/DB/lib_database.txt Although there are many better libraries, I still like my achievement here and I'm happy I can just call `database()->query_to_array_named_params()`, passing the SQL statement and the parameter list. – GolezTrol Mar 14 '11 at 23:03
  • http://devzone.zend.com/article/686 "Prepared statements provide developers with the ability to create queries that are more secure, have better performance, and are more convenient to write." Well, that sums it up. – GolezTrol Mar 14 '11 at 23:03
  • 1
    it's just empty blab again and no working example. That's what I am talking about – Your Common Sense Mar 14 '11 at 23:09
  • That zend's ranting is just not true. Prepared statements are NOT more secure nor have better performance nor more convenient to write. – Your Common Sense Mar 14 '11 at 23:11
  • 2
    The only one who's ranting is you. I already explained about the nuances of the 'better performance' statement, but could you please tell us *why* they are not more secure? The only thing you've essentially said is "No, not true", but without explanation, motivation or example. I've taken the effort to take my code, translate it and put it online. You can take a look at a real life example here: www.eftelist.nl. Now please come with a useful reply or just sod off and go compile yourself. – GolezTrol Mar 15 '11 at 07:35
  • 1. in real life, not imaginary one, we **do not** run the same query thousand times within the same connection. In the real life there is no speed benefit from preparing queries, but it's rather harm to speed, for doing 2 roundtrips to db server per query instead of one. 2. As you can see from this very topic, escaping is **secure enough**. So, how it can be that prepared statements are "more secure"? – Your Common Sense Mar 15 '11 at 10:31
  • 1. In real life there are most certainly queries that are executed multiple times. My company runs a webshop that has about a thousand hits per seconds and you can be sure there are duplicates. These are not within the same connection, but that doesn't matter. The point is that queries are cached on the server and the cached query can be used by another connection too. Having parameterized queries should result in less different queries in the cache. I already admitted this isn't yet the case in the current MySQL version. 2. Makes no sense. MD5 is secure enough, but SHA-1 is still more secure. – GolezTrol Mar 15 '11 at 11:46
-1

You are wrong. No injection possible here.

By following these three simple rules

  1. Client's encoding properly set by mysql_set_charset()
  2. Data being escaped using mysql_real_escape_string()
  3. And enclosed in quotes

you can be sure that no injection possible

Your Common Sense
  • 156,878
  • 40
  • 214
  • 345
  • @Igor Let's see. But I am sure you are under impression of some mistake. Urlencoding (%27) has nothing to do with database. With proper syntax no injection possible – Your Common Sense Mar 14 '11 at 21:07
  • I assure you that is the exact code I'm running and yet it seems I have SQL injection – Marcus Mar 14 '11 at 21:08