Why does MySQL allow fuzzy matches in SELECT queries?
Here's the story. I am testing running some security tests (using zaproxy) of a Laravel application (PHP framework) running with a MySQL database as the main data store.
Zaproxy reports a possible SQL injection for a POST request url with the following payload:
id[]=3-2&enabled[]=on
Basically, it's an AJAX request to enable / disable a specific feature in the list. Zaproxy scares the request: where value id
is 3-2
, there must be an integer - the id
item to update.
The problem is that this request works. It should fail, but the code is actually updating the element where id = 3
.
I am doing it as I should: the model is retrieved using an Eloquent method Model::find($id)
, passing in the value id
from the query (which after a bit of research was identified as string "3-2"). AFAIK, the Eloquent library needs to execute the query by binding the ID value to the parameter.
I tried to execute the request using the Laravel class DB
with the following code:
$result = DB::select("SELECT * FROM table WHERE id=?;", array("3-2"));
and got the string for id = 3
.
Then I tried the following query against my MySQL database:
SELECT * FROM table WHERE id='3-2';
and did to get the line where id = 3
. I also tried this with a different value: "3abc". It looks like any value prefixed with a number will retrieve the string.
So this is ultimately a MySQL issue. As far as I can tell, if I ask for a row that id = '3-2'
doesn't have a row with that exact id value, then I want it to return an empty result set.
I have two questions:
-
Is there a way to change this behavior? It seems to be at the database server level, so is there something in the database server configuration to prevent such a thing?
-
This looks like a major security issue to me. Zaproxy can add some arbitrary value and make changes to my database. Admittedly, this is a fairly minor issue for my application, and (maybe) only the values that will work will be number-prefixed values, but still ...
source to share
SELECT * FROM table WHERE id= ? AND ? REGEXP "^[0-9]$";
This will be faster than what I suggested in the comments above. Edit: Ah, I see you cannot change the request. This is then confirmed, you must sanitize the inputs in the code. Another very bad and dirty option if you are in a weird situation where you cannot change the query but can change the database is to change the id field to [VAR] CHAR.
source to share
I believe this is because MySQL will automatically convert your strings to numbers when compared to a numeric data type.
https://dev.mysql.com/doc/refman/5.1/en/type-conversion.html
mysql> SELECT 1> '6x';
-> 0
mysql> SELECT 7> '6x';
-> 1
mysql> SELECT 0> 'x6';
-> 0
mysql> SELECT 0 = 'x6';
-> 1
You want to really just put armor around MySQL to prevent comparison of such a string. Perhaps switch to a different SQL server.
source to share
Without rewriting a bunch of code, honestly the correct answer is
It's not a problem
Zaproxy even claims that he is possibly a SQL injection attack, which means he doesn't know! He never said "mmm, we deleted tables by passing xy-and-z to your query"
// if this is legal and returns results
$result = DB::select("SELECT * FROM table WHERE id=?;", array("3"));
// then why is it an issue for this
$result = DB::select("SELECT * FROM table WHERE id=?;", array("3-2"));
// to be interpreted as
$result = DB::select("SELECT * FROM table WHERE id=?;", array("3"));
You parameterize your queries so that Zaproxy disconnects from it.
source to share
Here's what I did:
First, I suspect that my expectations were a little unreasonable. I expected that if I was using parameterized queries, I would not need to sanitize my inputs. This is clearly not the case. While parameterized queries eliminate some of the most damaging SQL injection attacks, this example demonstrates that you still need to examine your inputs and make sure you are getting the data you want from the user.
So with that said ... I decided to write some code to make it easier to validate identity values. I added the following line to the application:
trait IDValidationTrait
{
/**
* Check the ID value to see if it valid
*
* This is an abstract function because it will be defined differently
* for different models. Some models have IDs which are strings,
* others have integer IDs
*/
abstract public static function isValidID($id);
/**
* Check the ID value & fail (throw an exception) if it is not valid
*/
public static function validIDOrFail($id)
{
...
}
/**
* Find a model only if the ID matches EXACTLY
*/
public static function findExactID($id)
{
...
}
/**
* Find a model only if the ID matches EXACTLY or throw an exception
*/
public static function findExactIDOrFail($id)
{
...
}
}
Thus, whenever I usually used a method find()
for my model class to retrieve the model, instead, I use either findExactID()
or findExactIDOrFail()
, depending on how I want to handle this error.
Thanks to everyone who commented - you helped me focus on my thinking and understand better what is going on.
source to share