Forum Moderators: coopster

Message Too Old, No Replies

Dealing with MySQL injection via query string

         

csdude55

5:35 am on Mar 31, 2022 (gmt 0)

WebmasterWorld Senior Member 10+ Year Member Top Contributors Of The Month



I've been dealing with a lot of attempts at MySQL injection. 99% of them are being blocked by Apache, but I've been logging query strings and came across some that went through when they shouldn't have.

Near the top of my script I have:

# I really need to ensure that they're numbers AND range from 18-99, but for now...
if ($_GET['fromage'] != intval($_GET['fromage']))
$_GET['fromage'] = 18;

if ($_GET['toage'] != intval($_GET['toage']))
$_GET['toage'] = 99;


Then I build the MySQL query:

$p_query = 'SELECT colA, colB FROM tableA WHERE ';

$p_query .= sprintf("age >= %s AND age <= %s ORDER BY updated DESC",
mysql_real_escape_string($_GET['fromage']),
mysql_real_escape_string($_GET['toage']));


The first bit of code SHOULD be enforcing that the variables are numbers, but I still see this:

SELECT colA, colB FROM tableA WHERE age >= 18 OR JSON_KEYS((SELECT CONVERT((SELECT CONCAT(0x773441,(SELECT (ELT(1196=1196,1))),0x376f4c)) USING utf8)))# j9Tm AND age <= 99 ORDER BY updated DESC


Any guesses on what they did to get the OR JSON... # j9Tm part to go through?

robzilla

7:43 am on Mar 31, 2022 (gmt 0)

WebmasterWorld Senior Member 10+ Year Member Top Contributors Of The Month



Any guesses on what they did to get the OR JSON... # j9Tm part to go through?

Misunderstanding of the function and lack of testing? :-)

$_GET['fromage'] = "18 OR JSON_KEYS((SELECT CONVERT((SELECT CONCAT(0x773441,(SELECT (ELT(1196=1196,1))),0x376f4c)) USING utf8)))";

var_dump(intval($_GET['fromage']));

Returns int(18).

var_dump(is_numeric($_GET['fromage']));

Returns bool(false) if the variable contains text. Work that one in, intval() is useless here.

robzilla

9:55 am on Mar 31, 2022 (gmt 0)

WebmasterWorld Senior Member 10+ Year Member Top Contributors Of The Month



So you might rewrite it like this:
if(!is_numeric($_GET['fromage']) || $_GET['fromage'] < 18 || $_GET['fromage'] > 99) {
$_GET['fromage'] = 18;
}

It only just clicked that it's "from age", and not fromage (as in omelette).

Also, maybe you're expecting mysql_real_escape_string() to do something here, but there aren't actually any characters (like quotes) to escape, so the "OR JSON_KEYS" etc. can come through without a problem (since it immediately follows the unquoted number). If you were to quote the variables you pass to MySQL, the injection would not have worked, so:
$p_query .= sprintf("age >= '%s' AND age <= '%s' ORDER BY updated DESC",
mysql_real_escape_string($_GET['fromage']),
mysql_real_escape_string($_GET['toage']));

Which would have resulted in the query:
SELECT colA, colB FROM tableA WHERE age >= '18 OR JSON_KEYS((SELECT CONVERT((SELECT CONCAT(0x773441,(SELECT (ELT(1196=1196,1))),0x376f4c)) USING utf8)))# j9Tm' AND age <= '99' ORDER BY updated DESC

Obviously there would be no record with that age value. Even though strictly speaking there shouldn't be quotes around numbers, MySQL will use type inference to figure out whether the value is a number or a string, and in this case it would have prevented the SQL injection.

Still, it's better to properly validate all input before passing it to MySQL (and then test those validations!).

csdude55

5:27 pm on Mar 31, 2022 (gmt 0)

WebmasterWorld Senior Member 10+ Year Member Top Contributors Of The Month



I built this script around 2008, and this is a new issue. So I'm guessing that something changed when I moved to the new server and updated PHP? I tested it on [sandbox.onlinephpfunctions.com...] and it worked as I expected, though, so I was kinda lost!

For the sake of my own education, can you explain why if ($_GET['fromage'] != intval($_GET['fromage'])) doesn't work?


It only just clicked that it's "from age", and not fromage (as in omelette).

Haha, I KNEW someone would say that! LOL I thought about changing the variable names for this post, but thought it best to leave it all as-is just in case the problem was a typo in the variable name or something.

I just now uploaded your suggestions, thanks a lot! I'll post back if it happens again.

NickMNS

8:58 pm on Mar 31, 2022 (gmt 0)

WebmasterWorld Senior Member 10+ Year Member Top Contributors Of The Month



and not fromage (as in omelette).

Fromage === Cheese in French!

robzilla

1:17 am on Apr 1, 2022 (gmt 0)

WebmasterWorld Senior Member 10+ Year Member Top Contributors Of The Month



For the sake of my own education, can you explain why if ($_GET['fromage'] != intval($_GET['fromage'])) doesn't work?

Oops, sorry, you caught a mistake of mine there. Sleeping on the job. That's a good point, I neglected to address that even though that's crucial to your issue, not intval(). The problem is actually the != which does a loose comparison between $_GET['fromage'] and intval($_GET[''fromage]).

Even though $_GET['fromage'] (with the SQL injection embedded) is initially a string, because it starts with a number PHP will loosely interpret it as a number and discard any non-numeric characters, so the value of $_GET['fromage'] is interpreted as 18, just like intval($_GET['fromage']). Trouble is, this only applies to the comparison, so the SQL injection remains in $_GET['fromage'] and bypasses your checks.

There's no such type conversion taking place if you use !== for an exact comparison. While intval($_GET['fromage']) will still be read as 18, $_GET['fromage'] will be compared as the string "18 OR JSON_KEYS etc".

Hope that makes sense.

So:

("18 and some text" !== 18) // interpreted as (string) "18 and some text" vs. (int) 18
("18 and some text" != 18) // interpreted as (int) 18 vs (int) 18

Note that some of this behavior toward numeric string seems to have changed in PHP 8: [php.net...]

I just now uploaded your suggestions, thanks a lot! I'll post back if it happens again.

It can't :-)

Fromage === Cheese in French!

Omelette du fromage! [youtube.com] First thing I always hear.

NickMNS

2:49 am on Apr 1, 2022 (gmt 0)

WebmasterWorld Senior Member 10+ Year Member Top Contributors Of The Month



I guess every other post will now read "Omelette de fromage!"





Sorry for taking this off topic...

csdude55

4:12 am on Apr 1, 2022 (gmt 0)

WebmasterWorld Senior Member 10+ Year Member Top Contributors Of The Month



Thanks for the explanation, @robzilla. I actually DID try using !== earlier, but it failed all tests; I assumed that, because it was a $_GET variable, it was coming in as a string instead of an integer?

But either way, so far so good.

Now, on to important things... the Dexter's Laboratory clip.

When I was in elementary school, one assignment was to learn all of the states in the US. I had failed to study, so I came up with this slick idea... sleep learning! I had a cassette player that you could set to loop, so I recorded myself saying all of the states, set it to loop, then tucked it under my pillow.

That was my first D, ever :'-(

That would have also been around 1985, so I'm horrified! Just horrified! To learn that Dexter copied that exact scene from my life! I'm absolutely convinced that I have microphones in the fillings in my teeth.

robzilla

8:04 am on Apr 1, 2022 (gmt 0)

WebmasterWorld Senior Member 10+ Year Member Top Contributors Of The Month



Omelette du fromage!

I actually DID try using !== earlier, but it failed all tests; I assumed that, because it was a $_GET variable, it was coming in as a string instead of an integer?

Yes, it's always a string, that's another good point I didn't address (-_-) So that's another reason why your old check is not a very helpful one.

Always go straight for the heart of the matter: is it a number (and nothing else)? is_numeric() does just that.

$_GET['fromage'] = "18";

var_dump(is_numeric($_GET['fromage'])); // true
var_dump(is_int($_GET['fromage'])); // false

$_GET['fromage'] = "18 AND JSON_KEYS etc";

var_dump(is_numeric($_GET['fromage'])); // false
var_dump(is_int($_GET['fromage'])); // false

I had a cassette player that you could set to loop, so I recorded myself saying all of the states, set it to loop, then tucked it under my pillow.

I'm surprised you could sleep at all!

lucy24

4:57 pm on Apr 1, 2022 (gmt 0)

WebmasterWorld Senior Member 10+ Year Member Top Contributors Of The Month



Looking at the topic title alone, my first question would have been: Are query strings--legitimate ones, I mean--part of the site’s visible URLs, or are they only used internally?

csdude55

6:20 pm on Apr 1, 2022 (gmt 0)

WebmasterWorld Senior Member 10+ Year Member Top Contributors Of The Month



@lucy24, yes, they are visible. I built it in 2008, I think, and at the time I liked the look of having readable variables in the URL. Plus it made it easy if I wanted to change something in the future.

It looks like @robzilla's solution fixed my immediate issue, though, so it's all good :-D There's a deeper issue, though, of the would-be cracker getting a 403 but it's still spiking the load on my server. I installed mod_security2 but it's too soon to know if it has helped. I'm still thinking about moving my checks from Apache config to PHP, then adding evil IPs to the firewall's block list.

topr8

9:46 am on Apr 4, 2022 (gmt 0)

WebmasterWorld Senior Member 10+ Year Member Top Contributors Of The Month



i've just a couple of things to add.
given that injection attempts usually try to inject a fair bit of text, i usually test the string length as part of any function i have for testing/cleaning querystrings.
so in this case the string length should never be longer than 2.
it isn't perfect in itself but is an added layer of protection.

also i always build queries (in php) using prepared statements, again it adds an extra layer of security.

(finally, i believe you shouldn't be sending that kind of query to the mySQL server from php anyway, you should build stored procedures on the server and only ever call them ... effectively sending parameters to the db server, which the server checks as well)