4

I am running a small website where I have published some articles. I am also very interesed in security topics and tried sqlmap on my website. It found the database name, so my question is how it could find it.

The website is very basic and does just contain a very small number of php files and I have just tested on the index.php file:
sqlmap.py -u http://myurl.com/index.php?id=5 --dbs
In the index.php I have used the php functions mysql_real_escape_string and stripslashes to clean the string before using it in the SQL question and if the visitor tries to mess with the id in the URL, then I just reload the index.php:

$id = $_GET['id'];
if (get_magic_quotes_gpc()) {
    $id = stripslashes($id);
}
$id = mysql_real_escape_string($id);

$sql = "SELECT * , COUNT( * ) AS amount
            FROM articles
            WHERE id = $id
            GROUP BY id";
$result = mysql_query($sql) or die(header("Location: index.php"));

So how on earth could sqlmap.py get the database name out of the above information?

Could you please explain how it is possible and also provide an example?

Rox
  • 801
  • 3
  • 10
  • 17
  • 1
    I'm not going to answer because I don't have anything specific. My guess is sqlmap uses some tricks the escape function doesn't take care of. I use a framework when I code to take care of this stuff. – k to the z Jan 26 '12 at 19:21
  • 2
    that is very obvious sql injection. – rook Jan 26 '12 at 23:07
  • 1
    .. and also a common mistake. See http://stackoverflow.com/questions/110575/do-htmlspecialchars-and-mysql-real-escape-string-keep-my-php-code-safe-from-inje/110576#110576 and http://security.stackexchange.com/questions/10974/sql-injection-information – Cheekysoft Jan 27 '12 at 09:54

4 Answers4

14

Always use parameterized queries, string concatenation leaves room for error, stop leaving that room (they also look a lot nicer).

Also, because id is an int, I don't need quotes to inject:

$id = '0; Drop Table myTable;--';

$sql = "SELECT * , COUNT( * ) AS amount
        FROM articles
        WHERE id = 0; Drop Table myTable;--
        GROUP BY id";

I like to use is_numeric to check if it's a number and kick them out of it isn't.

Edit:

PDO provides preparation of SQL queries: http://php.net/manual/en/pdo.prepare.php

So for example:

$sql = "SELECT * , COUNT( * ) AS amount
    FROM articles
    WHERE id = :id
    GROUP BY id";
$sth = $dbh->prepare($sql);
$sth->execute(array(':id' => $id));
StrangeWill
  • 1,603
  • 8
  • 13
4

First off, you're taking the right steps by not just assuming that your code is secure, but by actually testing it.

Secondly, Sqlmap will give you some levels of details as to exactly how it arrived at an SQLi point. Try some of the following when you run it:

sqlmap.py -v 3 --dbs -u http://yoursite.com/index.php?id=5

The -v 3 switch will cause it to print out the exact payload injected. You should also be able to see this in your web server's access and error logs as well.

Third, looking at your code snippet, you might want to try:

(int)$_GET['id'] 

But if you can maybe give us some more detail on what sqlmap is saying it's injection point is, such as timed, blind, union, boolean, etc. we could get into more detail.

SeeYouInDisneyland
  • 1,428
  • 9
  • 20
neil
  • 478
  • 4
  • 8
  • Ok, sqlmap printed this: sqlmap identified the following injection points with a total of 0 HTTP(s) requests: --- Place: GET Parameter: id Type: boolean-based blind Title: AND boolean-based blind - WHERE or HAVING clause Payload: id=2 AND 597=597 --- But when I add "AND 597=597" to the URL in the browser, nothing happens, so how can sqlmap get the error if the browser does not? – Rox Jan 26 '12 at 19:57
2

All the answers are ok, but in your particular case you escaped the number as a string (mysql_real_escape_string()), but you didn't enclose it in single quotes as a string should be.

If, instead of:

$sql = "SELECT * , COUNT( * ) AS amount
        FROM articles
        WHERE id = $id
        GROUP BY id";

you used:

$sql = "SELECT * , COUNT( * ) AS amount
        FROM articles
        WHERE id = '$id'
        GROUP BY id";

there would be no SQL injection a decreased risk of SQL injection. But it's always better to:

  • use prepared statements
  • use strict input validation - in your case you should be casting id to integer.
Cheekysoft
  • 1,267
  • 1
  • 9
  • 12
Krzysztof Kotowicz
  • 4,068
  • 20
  • 30
  • 1
    It is incorrect, in this example, to say that "there would be no sql injection"; However there will be a smaller attack surface. As mysql_real_escape_string is just a string-to-string conversion function, and unfortunately not a perfect one, this code could be vulnerable to various forms of malformed multi-byte character attacks, depending on the active configuration in use. It is possible to construct malformed characters which are translated differently by PHP and the DBMS. – Cheekysoft Jan 27 '12 at 09:51
  • 1
    This can have dire consequences on query optimization - be careful. A better solution is to use regexes to check numeric values are numeric - otherwise quote them. – symcbean Jan 27 '12 at 10:06
  • @Cheekysoft true, there could be sql injection if database used a vulnerable multibyte charset like Shift-JIS, but it's highly unlikely. Still, good catch, thanks. – Krzysztof Kotowicz Jan 27 '12 at 23:11
  • Also the mismatching types (wrapping an int in quotes to make it a string) requires you to be running MySQL in a very liberal configuration which generally you don't want your database just opaquely interpreting types back and forth for you. – StrangeWill Sep 26 '19 at 05:53
2

The short answer in this particular situation, and because the $id is always going to be a number, is as Neil stated, to filter the input to make sure it is always going to be an integer no matter what.

Example concept:

$id = ( false !== ( int )$_GET[ 'id' ] >= 0 ) ?
        ( int )$_GET[ 'id' ] :
        die( header( "Location: ./index.php" ) );

Something like that would make sure that $id can only ever be an integer and not less than 0 (i.e. not -1). What you do with a failed id is up to you, in this example the page is redirected to the index.php.

In this example above, the following request of

id=55+union+select+1,2,3,4,5,6,7,8,9,10,11,12,13

Would result in $sql being:

SELECT * , COUNT( * ) AS amount FROM articles
WHERE id = 55 GROUP BY id

instead of what the current code would return which is:

SELECT * , COUNT( * ) AS amount FROM articles
WHERE id = 55 union select 1,2,3,4,5,6,7,8,9,10,11,12,13 GROUP BY id
Gilles 'SO- stop being evil'
  • 50,912
  • 13
  • 120
  • 179
Taipo
  • 189
  • 4