Its a common misconception that as MongoDB does not use SQL it is not vulnerable to SQL injection attacks. PHP uses objects rather than SQL to pass queries to the MongoDB server; for example the following script selects an item form MongoDB where the username equals ‘bob’ and the password equals ‘password’.
$collection->find(array(
"username" => $_GET['username'],
"passwd" => $_GET['passwd']
));
This is equivalent to the SQL syntax
mysql_query("SELECT * FROM collection
WHERE username=" . $_GET['username'] . ",
AND passwd=" . $_GET['passwd'])
In a normal SQL injection attack we can replace either of the two input parameters with a string such that the SQL query always returns true. e.g.
login.php?username=admin&passwd=" OR 1 --
That wont work with MongoDB; however if we can pass in an object to the PHP MongoDB driver we could alter the query in a similar fashion. Luckily PHP provides us with a way to pass objects as GET or POST parameters:
login.php?username=admin&passwd[$ne]=1
This creates the MongoDB query
$collection->find(array(
"username" => "admin",
"passwd" => array("$ne" => 1)
));
Which is the equivalent to the following SQL statement which, unless the password is “1″ will always return true.
mysql_query("SELECT * FROM collection
WHERE username="admin",
AND passwd!=1
The solution is to ensure your variables are properly typed before they are passed into the MongoDB driver. The following code is not vulnerable to MongoDB injection:
$collection->find(array(
"username" => (string)$_GET['username'],
"passwd" => (string)$_GET['passwd']
));
Hey Phil, thanks for the post. Would this work as well than? Note the double quotes.
Ah answered my own question. So you could do something like the following as well:
than…
Your code would work but this would also work and means you don’t have to declare $username.
Either way as long as the input to mongodb remains a string your safe :)
Nice catch! Just a suggestion though: never access $_GET directly. Always use a getter function, to which developer MUST specify the expected format. This approach solves most of the web application security problems.
For instance:
“username” => get(‘username’, GET_STRING, ‘/^[a-zA-Z0-9]+$/’)
(regex is optional)
Or:
“username” => get_string(‘username’, ‘/^[a-zA-Z0-9]+$/’)
Function get() should be the only way to access $_GET. You should even rename $_GET to make sure developers obey this convention.
My 0.02 EUR.
well Andrew ,
But where can we find the code of the functions :
get_string()
and/or
get()
I can’t find them in the http://php.net/manual/ !
Keep in mind that in PHP 5.4 casting an array to string will throw a notice.
@witty: well, you have to make them yourself. I am currently not aware of any PHP framework (other than my own ;) that would tackle the problem in this way.
But it should be really easy to do. The tricky part is making this option easy to use and all other options as difficult as possible – so you are never tempted to make a “shortcut”.
@Wim: that’s ok, array is not supposed to be there anyway. Notice is generated only for malicious users and (in properly configured system) is not even displayed – it just ends up in your logs. No harm done.
Andrew: Your point ain’t great. Even if you write your own function you’d need to access $_GET sooner or later. Can’t see why it would make any difference?
Will this happen if I use $_POST in my mongo query instead of GET?
yes
you mean nosql injection? :)