6

Possible Duplicate:
What does mysql_real_escape_string() do that addslashes() doesn't?

I have been reviewing articles on how/why PHP's addslashes function is vulnerable to sql injection. Everything I have read says there are problems with specific mysql encoding types (default-character-set=GBK), or there are problems if magic_quotes are enabled. However, I have been unable break out of the addslashes() function in this scenario and do something malicious - such as login as an administrator.

    $user = addslashes($_POST['user']);
    $pass = sha1($_POST['pass']);
    $sql = "SELECT * FROM admins WHERE user = '".$user."' AND `pass` = '".$pass."'";

    $nums = mysql_num_rows(mysql_query($sql));

    if($nums==1){
    $_SESSION['admin_user'] = $user;
    $_SESSION['admin_pass'] = $pass;

This is a (minor) security audit for a client and I will recommend that they utilize PDO, but I need to display their current vulnerability.

References:

Community
  • 1
  • 1
k10
  • 109
  • 2
  • 6
  • Well, I'll just say that just because you're unable to exploit a function in your own code doesn't mean nobody else can. – BoltClock Dec 01 '11 at 10:32
  • 2
    `I will recommend that they utilize PDO` that's a fair recommendation, but in this specific piece of code, it will suffice to use the *right* sanitation function instead of addslashes: `mysql_real_escape_string()`. – Pekka Dec 01 '11 at 10:32
  • 2
    Shiflett shows a full exploit in his blog entry. The code you show above doesn't seem to be following that example. – Pekka Dec 01 '11 at 10:33
  • @pekka using normal query() and execute() of PDO it self will solve the issue or i should use prepared statements? – Mithun Satheesh Dec 01 '11 at 10:38
  • @mithun no, PDO's query() alone will not solve the problem. When using PDO, you need to use prepared statements so all data is sanitized. If you just feed a full query containing raw data to PDO, it's as unsafe as using `mysql_query()` with no sanitation – Pekka Dec 01 '11 at 10:39
  • As a side observation, *this specific query* could happen to be safe because I don't think there is a way to override the `AND pass` part, and that is sanitized because of the `sha1()` call. But using `addslashes()` is still wrong and dangerous practice and they should change it. – Pekka Dec 01 '11 at 10:41
  • @pekka - agree with everything you've said, and maybe this guy just isn't vulnerable. that's my question. i can't see a way out of it, so i am asking for some help. (PS: thanks for all the responses) – k10 Dec 01 '11 at 10:43
  • Well, applying addslashes to UTF-16/UCS-2 encoded string will positively mess it up. If it's little-endian, then you can also turn that into a vulnerability. But again - it's a specific charset. I cannot think of a way that UTF-8 could be thwarted. – Vilx- Dec 01 '11 at 10:43
  • @pekka - i definitely have enough to argue for the usage of PDO, but they asked for a vulnerability display and i just grabbed the first query i saw and figured that you guys would be able to give me a hand. knowing a little bit more about the addslashes vulnerabilities is a good thing - again, thanks for your assistance. – k10 Dec 01 '11 at 10:46
  • @stevek no problem. I made the comment an answer. If they are security-unconscious enough to use `addslashes()` in queries, go check whether there are any uses of `int` values without surrounding quotes: `SELECT * FROM customers WHERE id = $value`. Those are easy to mess with: Just add `999 OR id = {insert some other customer's ID here}` - `addslashes()` will add no protection here – Pekka Dec 01 '11 at 10:48
  • Ahh, wait, overlong UTF-8 sequences could be used to circumvent addslashes, though I don't know whether MySQL would accept them or not. It is borderline incorrect UTF-8 after all. – Vilx- Dec 01 '11 at 10:49
  • An apostrophe is one byte 0x27 in UTF-8 (the same as ASCII, and addslashes escapes that correctly). However it can also be encoded as 0xC0 0xA7. This is invalid, because the spec says that the shortest possible representation must be used. Bit if MySQL is forgiving enough... Have to check that though. I'm afraid I don't have MySQL handy at the moment. :( – Vilx- Dec 01 '11 at 10:55
  • possible duplicate of [What does mysql_real_escape_string() do that addslashes() doesn't?](http://stackoverflow.com/q/534742/), [Examples of SQL Injections through addslashes()?](http://stackoverflow.com/q/860954/) – outis Jul 13 '12 at 12:11

4 Answers4

3

Is PHP's addslashes vulnerable to sql injection attack?

Yes. With the conditions mentioned in the article you linked to.

I need to display their current vulnerability.

I doubt you can display this one, as it seems the client's encoding is not the renowned GBK.

Though, there most likely exists other possible vulnerabilities, just because people tend to misuse an escaping function, taking it wrong.

But I can assure you that as long as your client's encoding either single-byte one or UTF-8, and as long as this function used properly (as in the code you posted)- there would be no injection possible.

Your Common Sense
  • 156,878
  • 40
  • 214
  • 345
  • thanks for the response. if the client was setup with the same conditions in the article this would be an easy one. unfortunately i don't see a vulnerability in the code above with their current setup. i appreciate your response. – k10 Dec 01 '11 at 10:47
  • 1
    I've just posted an addition assuring you that there is no injection possible in this code. – Your Common Sense Dec 01 '11 at 10:51
3

For your particular case, it does not seem that it is as easy to perform SQL injection, but a common thing to try is something like, if i enter a unicode null variable? like \0? Will it break the script and return everything? Most likely not.

So thing is, you do not always need slashes to perform SQL injection. Some SQL can be written so horrible wrong, heres an example

"SELECT * FROM admins WHERE id = $id"

If $id is a number, its perfectly valid SQL, and you perform addslashes on $id, (who would do that anyway?). But for this specific case, all you need for SQL injection is 1 OR 1=1 making the query look like

"SELECT * FROM admins WHERE id = 1 OR 1=1"

There is no way addslashes or magic_quotes could protect you against that sort of stupidity.

To get back to the question at hand, why would anyone in their right mind ever use GBK over something like UTF-8 or UTF-16?

Jan Dragsbaek
  • 8,078
  • 2
  • 26
  • 46
3

Shiflett shows a full working exploit in his blog entry. The code you show above doesn't seem to be following that example as it's not using the character set that exhibits the vulnerability. Still, the hole definitely exists.

Even if it happens to be safe in the specific scenario, the practice of using addslashes() is still dangerous and Shiflett's article should give you enough material to argue with, even though the circumstances the exploit requires are very esoteric, and they're not entirely trivial to reproduce.

If your client doesn't accept the danger without seeing a live exploit on their specific system, then they're not worth doing a security audit for.

Pekka
  • 442,112
  • 142
  • 972
  • 1,088
  • +1 and I totally agree with the last sentence. The very fact that `addslashes` has _any_ proven exploits should be enough to raise some eyebrows. If your client isn't willing to take advantage of techniques known to improve security, then they simply aren't that concerned with security. – Herbert Dec 01 '11 at 10:57
  • 1
    Almost EVERY reasonably popular piece of software has some proven exploits. So that fact **alone** should not be serious enough to ban it outright. That way we arrive at the classical "100% security = 0% usability". That said, it should be enough to get them interested in a deeper analysis of whether that exploit affects **THEIR** specific case. It could easily enough turn out that they are (consistently) using a character set which is invulnerable to these problems (say, Windows-1252). In that case it's reasonable that it becomes just another line in their programming guidelines. – Vilx- Dec 02 '11 at 09:48
-4

I'm not sure why you would want to use addslashes over mysql_real_escape_string() but that's entirely your choice I suppose.

addslashes() does exactly what it says: Quote string with slashes

Example #1 An addslashes() example
<?php
$str = "Is your name O'reilly?";

// Outputs: Is your name O\'reilly?
echo addslashes($str);

But some SQL attacks can be performed without quotes (certain shell injections and some blind SQL injections).

For this reason I would personally use mysql_real_escape_string() over addslashes() for securing your data in THIS case.

Also consider using sprintf() for your sql queries as it makes it slightly more secure:

$query = sprintf("SELECT `username`,`password`
        FROM admins 
        WHERE user = '%s' 
        AND `pass` = '%s'", 
    $user, $pass);

It makes it more secure as it won't allow any other data-types than the ones that you have given.

%d = digit %s = string, etc.

I hope this helps.

Andrew Brēza
  • 7,705
  • 3
  • 34
  • 40
DarkMantis
  • 1,510
  • 5
  • 19
  • 40
  • What are these "shell and blind injections" you are talking about? – Your Common Sense Dec 01 '11 at 10:51
  • Are you implying that you wish to know what they are, or which ones do not require quotes? – DarkMantis Dec 01 '11 at 14:49
  • Frankly, I am quite sure that no attack possible as long as you have your data quoted and escaped. all that blind stuff is just a variations of the theme. But once injection is impossiuble, no variant will succeed. – Your Common Sense Dec 01 '11 at 17:48
  • If all data is sanitized correctly then you'd be correct that no attack would be possible. However, to inject a shell (only works on Windows based servers) you do not need any form of quotes to do so. – DarkMantis Dec 13 '11 at 14:48