15

I've setup a test environment for running some SQL Injection against my code and learning how to defend against it. I can bypass the login form using the following in the password field:

' OR username = 'admin

Which gives me the query:

SELECT * FROM customer_data WHERE username = '' AND password = '' OR username = 'admin'

This works fine but I'm having trouble with dropping a table. I've tried various attempts along the lines of inserting something like this in the password field:

OR '1' = '1'; DROP TABLE temp; --

I created a table called temp just to try it against but nothing I can think of has worked. If I login to the PHP MyAdmin (same credentials as the page is using) I can manually insert the query that would be constructed above and it works just fine.

TildalWave
  • 10,801
  • 11
  • 45
  • 84
Scott Helme
  • 3,178
  • 3
  • 21
  • 32
  • 1
    @RohanDurve-Decode141 - I had to rollback to previous revision, because your edit actually solves OP's problem presented in the question. Well, at least partially (another `'` would be missing before `; DROP TABLE temp; --`. And the second `DROP TABLE` query statement won't be accepted anyway. Please, post suggested corrections as an answer instead. Thanks! ;) – TildalWave Jul 22 '13 at 08:02
  • 1
    Most MySQL connectors don't support stacked queries, which is in general a good thing. There is one write-construct you can typically inject from a `WHERE` which is `SELECT ... INTO OUTFILE`, which you can cause some damage with if the DB user is badly permissioned. – bobince Jul 22 '13 at 08:33
  • There are a lot of examples/tutorials/demos online that show how to drop tables by stacking a query such as the attempt I made above. How would you actually allow this to happen? I am currently using mysql_query() or mysqli_query(). – Scott Helme Jul 22 '13 at 08:44
  • @ScottHelme neither `mysql_query()` nor `mysqli_query()` supports multiple queries. What you're looking for is `mysqli_multi_query()` – Adi Jul 22 '13 at 08:50
  • Yeah I have just seen that in the docs after searching. What I don't get though is why someone would put that in their code. Would it not be best to construct multiple queries than run the risk? Or is people dropping tables just not that common? – Scott Helme Jul 22 '13 at 08:55
  • 1
    @ScottHelme: in PHP's `mysql` connector, you're right, they typically wouldn't. But for other lanaguges/DBMSs/connectors the default (or only) option is often to permit stacked queries. The stacked `DROP TABLE` attack is not really the most representative example of an SQL injection, but it's one that is the least app-specific and most easily understood, which is why it's quoted so much. – bobince Jul 31 '13 at 11:29

3 Answers3

34

Answering your question

mysql_query() doesn't support multiple queries as documented:

mysql_query() sends a unique query (multiple queries are not supported) to the currently active database on the server that's associated with the specified link_identifier.

Which means that DROP TABLE temp; -- is never executed.
It is although possible if you use mysqli::multi_query or using PDO.

Best practice

Warning ?
You see that big red warning box ? The mysql extension is in process of deprecation and will throw warnings as of PHP 5.5.0. There are even plans to drop it completely in PHP 5.6 or PHP 6.0source. Note that mysql_ isn't broken as Ángel González stated:

The extension is not broken. The problem is the bad usage. It can be used safely, and good developers have been doing so for ages, by creating php wrappers. In magic quotes, the work has been the opposite. The developers had been detecting the feature in php and disabling it.

The new standard
The new standard is MySQLi or PDO. I won't compare the two extensions here but there are really a bunch of good features especially from PHP 5.3+ which will save you time and efforts. Note that by using MySQLi/PDO won't protect you per se from SQL injections. The best option would be to use prepared statements. The data is sent separately from the query thus making it impossible to inject values. This is well explained by Anthony Ferrara in this video.

Be careful
But wait "impossible" ? That sounds just too great :)
Say for example we have two groups: group1 & group2. There is a certain php file deleteUser.php getting an id from $_GET.
The prepared statement looks like this: DELETE FROM users WHERE id = ?. When the query is made with $_GET['id'] the user with ID = $_GET['id'] will get deleted. Hey, but that means that users from group1 could delete users from group2 by using that ID which isn't intended. So we may edit the query in something like DELETE FROM users WHERE id = ? AND group = ? and sending the user's group name he's in along with the query.
Short: prepared statements won't protect you from logic flaws.

Multi query ?
You don't need mutli queries. If you want to do two things then do it separately, it will give you more control over your queries and make your code more readable.

On another note:
If you're dynamically making and droping tables you're most likely doing it wrong. You should design your database in such a way that this isn't needed. Of course there may be exceptions but they are mostly busted since you may then look for a NoSQL solution (other db engine).

TL;DR

  • Stop using mysql_ functions and step over to MySQLi or PDO
  • Use prepared statements, note that it will not protect you from attacks if you don't secure the logic behind it
  • Don't use multi queries
  • Make a good DB design
  • If you want to train your hacking skills, you may check Vulnerable OS's?
Community
  • 1
HamZa
  • 1,370
  • 1
  • 15
  • 19
10

It might be that the function you use in PHP does not allow stacked queries or because the user which is configured to perform the queries does not have drop table privileges. I suggest you log all the output and exceptions thrown by the application. It could give a clue where your problem lies.

EDIT 1: Instead of executing the query, print it to the screen.

Lucas Kauffman
  • 54,169
  • 17
  • 112
  • 196
  • The error that it's throwing is a syntax error, You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near 'DROP TABLE temp; --'' at line 3. – Scott Helme Jul 22 '13 at 07:36
  • The user has sufficient privileges as I can login to the PHP MyAdmin page with the same credentials and drop the table there. – Scott Helme Jul 22 '13 at 07:37
  • Updated my answer. – Lucas Kauffman Jul 22 '13 at 07:42
  • For the input: ' OR '1'='1'; DROP TABLE temp; -- I'm getting the query: SELECT * FROM customer_data WHERE username = '' AND password = '' OR '1'='1'; DROP TABLE temp; --'NULL The apostrophe and NULL on the end are throwing me there... – Scott Helme Jul 22 '13 at 07:49
  • Try ' OR '1'='1'; DROP TABLE temp; # – Rohan Durve Jul 22 '13 at 07:53
  • Thanks for your input guys. It seems I missed something in the docs and mysql_query() doesn't support multiple queries. – Scott Helme Jul 22 '13 at 08:03
  • 2
    That's what I said, stacked queries aren't supported by mysql_query() :) – Lucas Kauffman Jul 22 '13 at 08:03
4

This was an oversight on my part after some late night reading of the documentation!

mysql_query() sends a unique query (multiple queries are not supported) to the currently active database on the server that's associated with the specified link_identifier.

Scott Helme
  • 3,178
  • 3
  • 21
  • 32