6

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?

Improve this question
4
  • 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
    Commented Aug 24, 2016 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
    Commented Aug 24, 2016 at 17:07
  • 1
    Please use prepared statements. Also, Unicode/different character set tricks will bypass this every time.
    – Mark Buffalo
    Commented Aug 24, 2016 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
    Commented Aug 24, 2016 at 19:34

1 Answer 1

Reset to default
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.

Improve this answer
3
  • 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
    Commented Aug 24, 2016 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.
    – 700 Software
    Commented Aug 24, 2016 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
    Commented Aug 25, 2016 at 22:04

You must log in to answer this question.

Not the answer you're looking for? Browse other questions tagged .