5

I tried to find some SQL injection exploits that could bypass functions that should prevent sql injection vulnerabilities, for example mysql_real_escape_string. I found an exploit and the author described the vulnerability and posted the vulnerable code:

<?php
 if(isset($_GET['pagename'])){
  $name=$_GET['pagename'];
  $query=sprintf("SELECT* FROM ".PREFIX."page WHERE
  pagename = '%s' AND publish = 'Y'",$xx_con->real_escape_string($name));
}

But he did not describe how to bypass the protection, i.e. the function real_escape_string. Is there a real SQLi vulnerability in the code and how could an attacker exploit it?

  • Except for a few multi-byte issues, escape string is secure if used correctly (it's not recommended though), which it is here, so there is no injection. My guess is that it's a fake advisory, maybe to get people to visit their blog, or to see which mailing lists accept advisories without actually checking them; or maybe the author was just mistaken (their other current LFI/RFI advisories for example seem to assume that register globals - a setting that was removed from PHP 4 years ago - is on, but do not see a need to even mention that fact). – tim Aug 24 '16 at 17:06
  • Of course, there could always be issues in the combination of sprintf and real_escape_string, possibly with some specific character set, but if that were the case, it would be mentioned in any decent advisory. Without any further evidence, there is no reason to assume that this is the case here. – tim Aug 24 '16 at 17:07
  • 1
    Please use prepared statements. Also, Unicode/different character set tricks will bypass this every time. – Mark Buffalo Aug 24 '16 at 18:32
  • 1
    @MarkBuffalo every time, not just in some very specific corner cases (which would eg also affect PDO prepared statements)? If that's the case that would probably be a great answer (if you have the time to write it up); I'd definitely be interested in that. – tim Aug 24 '16 at 19:34
2

Update: see comments below regarding proper implementation.


Original answer, before knowing this was officially documented:

The use of function real_escape_string with substitution %s within an already quoted string '%s' looks highly suspect.

There could well be a vulnerability if

  • if real_escape_string not designed correctly,
  • or not used in accordance with its documentation. (mis-used, perhaps with extra quotes ' or something else)

I would highly recommend using a more standard method of solving SQLi in PHP. One that involves ? substitution is best. However there may be another sound encoding method available. Such methods usually add their own quotes internally, in which case adding quotes ' in your code would be unnecessary.

I did notice one other thing about that code. Where does PREFIX come from? If this is user-provided then you obviously have an SQLi vulnerability. On the other hand, if it is simply a hard-coded or well-sanitized value then you are OK with that one.

  • 1
    The combination of real_escape_string and sprintf is also used in the PHP documentation. I'm not saying that it is a good idea - escaping isn't a good idea in itself - or that the PHP documentation never contains vulnerable code, but I would assume that such an issue would have been more widely publicized over the years. – tim Aug 24 '16 at 17:19
  • Thanks @tim, Note that mysql_real_escape_string was deprecated in PHP 5.5.0, and it was removed in PHP 7.0.0. That's not to say it is vulnerable (when properly used), but it's a good reason to migrate to a more modern solution. – Bryan Field Aug 24 '16 at 18:06
  • 1
    Keep in mind mysql_real_escape_string was deprecated and removed only for the old mysql extension. This is because mysql was phased out in favor of mysqli and pdo. So real_escape_string is still in PHP even as of PHP 7. However, it can be exploited if mysqli_set_charset is not set or unicode character enforcement is not properly set up. Ideally, the best practice is to switch to prepared statements. – Bacon Brad Aug 25 '16 at 22:04

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.