4

I have an existing application which used to use the deprecated mysql_* functions to perform database queries. I have since changed most database access (and all that has user input) to PDO, so I believe I am relatively safe from injection attacks. However, I was wondering how one would perform an injection attack on the previous code just so I can demonstrate how unsafe it is should the need arise.

I have a link in the format:

http://localhost/api/view.php?id=

Which is then passed, unsanitized, into the select function below:

$db->select('invitations','Replied, Response, Registered',null,"Id = '".$id."'");
$res = $db->getResult();

I then do a var_dump() to see the result.

I have tried something like:

  • http://localhost/api/view.php?id=<id>' => array(1) { [0]=> string(185) "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 ''<id>''' at line 1" }

  • http://localhost/api/view.php?id=<id>" or 1=1" => array(0) { }

But it doesn't seem like those types of queries would be successful. Ideally, I would want to be able to do something severe. I was hoping for something like '); DROP TABLE users;--, but I cannot seem to make anything of the sort occur.

Here is the full select function:

public function select($table, $rows = '*', $join = null, $where = null, $order = null, $limit = null){
    // Create query from the variables passed to the function
    $q = 'SELECT '.$rows.' FROM '.$table;
    if($join != null){
        $q .= ' JOIN '.$join;
    }
    if($where != null){
        if (is_array($where)) {
            $filter = $where;
            $where = " 0 = 0 ";
            $qs = "";
            for ($i=0;$i<count($filter);$i++){
                switch($filter[$i]['data']['type']){
                    case 'string' : $qs .= " AND ".$filter[$i]['field']." LIKE '%".$filter[$i]['data']['value']."%'"; Break;
                    case 'list' :
                        if (strstr($filter[$i]['data']['value'],',')){
                            $fi = explode(',',$filter[$i]['data']['value']);
                            for ($q=0;$q<count($fi);$q++){
                                $fi[$q] = "'".$fi[$q]."'";
                            }
                            $filter[$i]['data']['value'] = implode(',',$fi);
                            $qs .= " AND ".$filter[$i]['field']." IN (".$filter[$i]['data']['value'].")";
                        }else{
                            $qs .= " AND ".$filter[$i]['field']." = '".$filter[$i]['data']['value']."'";
                        }
                        Break;
                    case 'boolean' : $qs .= " AND ".$filter[$i]['field']." = ".($filter[$i]['data']['value']); Break;
                    case 'numeric' :
                        switch ($filter[$i]['data']['comparison']) {
                            case 'eq' : $qs .= " AND ".$filter[$i]['field']." = ".$filter[$i]['data']['value']; Break;
                            case 'lt' : $qs .= " AND ".$filter[$i]['field']." < ".$filter[$i]['data']['value']; Break;
                            case 'gt' : $qs .= " AND ".$filter[$i]['field']." > ".$filter[$i]['data']['value']; Break;
                        }
                        Break;
                    case 'date' :
                        switch ($filter[$i]['data']['comparison']) {
                            case 'eq' : $qs .= " AND ".$filter[$i]['field']." = '".date('Y-m-d',strtotime($filter[$i]['data']['value']))."'"; Break;
                            case 'lt' : $qs .= " AND ".$filter[$i]['field']." < '".date('Y-m-d',strtotime($filter[$i]['data']['value']))."'"; Break;
                            case 'gt' : $qs .= " AND ".$filter[$i]['field']." > '".date('Y-m-d',strtotime($filter[$i]['data']['value']))."'"; Break;
                        }
                        Break;
                }
            }
            $where .= $qs;
        }
        $q .= ' WHERE '.$where;
    }
    if($order != null){
        $q .= ' ORDER BY '.$order;
    }
    if($limit != null){
        $q .= ' LIMIT '.$limit;
    }
    // Check to see if the table exists
    if($this->tableExists($table)){
        // The table exists, run the query
        $query = @mysql_query($q);
        if($query){
            // If the query returns >= 1 assign the number of rows to numResults
            $this->numResults = mysql_num_rows($query);
            // Loop through the query results by the number of rows returned
            for($i = 0; $i < $this->numResults; $i++){
                $r = mysql_fetch_array($query);
                $key = array_keys($r);
                for($x = 0; $x < count($key); $x++){
                    // Sanitizes keys so only alphavalues are allowed
                    if(!is_int($key[$x])){
                        if(mysql_num_rows($query) > 1){
                            $this->result[$i][$key[$x]] = $r[$key[$x]];
                        }else if(mysql_num_rows($query) < 1){
                            $this->result = null;
                        }else{
                            $this->result[$key[$x]] = $r[$key[$x]];
                        }
                    }
                }
            }
            return true; // Query was successful
        }else{
            array_push($this->result,mysql_error());
            return false; // No rows where returned
        }
    }else{
        return false; // Table does not exist
    }
}

How would I be able to perform an SQL injection?


EDIT:

The value is received via browser using $_GET['id']. There is, in fact, a second value which is not used in an SQL query but a switch statement. So, the full request looks like this:

http://localhost/api/view.php?rsvp=<status>&id=<id>

That may help.

EDIT 2:

It may have been important to mention that the ids in question are in fact GUIDs, not numeric values. I ran all queries in the comments so far with a proper ID. Here are some results:

  • http://localhost/api/view.php?id=62FD23D8-B6C0-03F1-D45A-C9AC33C91774%27;%20drop%20table%20users;-- => array(1) { [0]=> string(166) "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 users;--'' at line 1" }

  • http://localhost/api/view.php?id=62FD23D8-B6C0-03F1-D45A-C9AC33C91774%27;select%20*%20from%20invitations;%27 => array(1) { [0]=> string(179) "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 'select * from invitations;''' at line 1" }


These following lines are the entirety of the code that makes use of $id:

// ...
if(isset($_GET['id'])) {
    $id = $_GET['id'];
    $db->select('invitations','Replied, Response, Registered',null,"Id = '".$id."'");
    $res = $db->getResult();
// ...
}
  • `$db->select('invitations', '1; DROP TABLE invitations;', ....);` without seeing where your user data is coming from and what happens to it before it reachs the `$db->select` its fairly difficult to craft anything more – pala_ Apr 21 '15 at 06:24
  • try http://localhost/api/view.php?id=';select * from judgeinvitations;' the starting and end single quote are required. – Prabin Meitei Apr 21 '15 at 06:27
  • @pala_ The user data is coming from the browser, i.e. `$id = $_GET['id'];`. It is not manipulated in any way before being used in `$db->select`. – user1456733 Apr 21 '15 at 06:28
  • @PrabinMeitei That results in a syntax error as well. – user1456733 Apr 21 '15 at 06:29
  • ah, so id is the only value that comes in? no others? and it just goes in to `WHERE id = $id` ? – pala_ Apr 21 '15 at 06:29
  • @pala_ Good question! Sorry for omitting: two values come in, `http://localhost/api/view.php?rsvp=&id=`. The first value is not used in any SQL queries, however, only a `switch` statement. I'll add this to the question. – user1456733 Apr 21 '15 at 06:30
  • and can you also add the format of the $where variable, as its passed to the function? i assume thats where the value from `id` is used – pala_ Apr 21 '15 at 06:31
  • The format is as shown above: `"Id = '".$id."'"`, which gets sent to the `select()` function unsanitized. Or did you mean something else? – user1456733 Apr 21 '15 at 06:33
  • 3
    ah no. i see it now. try `?id=1'; drop table users;--` – pala_ Apr 21 '15 at 06:35
  • Try localhost/api/view.php?id=0';select * from judgeinvitations;' The intention is to make the query select * from table where id = '$id' result in select * from table where id = '0';select * from judgeinvitations;'' – Prabin Meitei Apr 21 '15 at 06:40
  • Unfortunately, both of those resulted in syntax errors without executing anything! There is some sanitation being doing in `select`, in fact (line 13 in `select`), but I was hoping that there was some way to circumvent it. – user1456733 Apr 21 '15 at 06:44
  • can you print the resultant query from each of the above?, ie `$q` just before the query is executed? – pala_ Apr 21 '15 at 06:49
  • Sure, just a moment. Also, it may have been important to note but I completely forgot to mention: The `Id`s in question are *not* numeric IDs, but `GUID`s. Is an injection attack possible on such a table? **Note**: When trying the queries above, I used an actual ID that is in the Database. – user1456733 Apr 21 '15 at 06:51
  • 3
    1) In order to find exploits, it's pretty much a necessity to have access to the running code on a production system. Just having a code dump we cannot execute pretty much just forces us to guess. 2) mysql is not insecure per se, if applied properly it's fine. However, it's hard to know whether you're applying it properly or not without seeing the entire codebase. 3) Even if `'; DROP TABLE ..` doesn't work, just being able to perhaps add a condition and slightly change the meaning of the query may be enough for an attacker to glean useful information. – deceze Apr 21 '15 at 06:52
  • 3
    I don't think you are going to make `; drop anything` work as @deceze pointed out... just because `mysql_query` [does not allow multiple queries with `;` to be executed](http://stackoverflow.com/questions/11106719/php-multiple-mysql-commands-in-one-mysql-query-query) – β.εηοιτ.βε Apr 21 '15 at 07:03
  • @deceze That does make sense. I did not want to make it seem like it was an easy task (if it was I could have likely searched for a solution by now :) ). Then, would you say that the fact that extra special characters generate an error rather than being ignored is a security concern? I would really like to have a retort to "if it's not broken, don't fix it" in this case, besides deprecation. – user1456733 Apr 21 '15 at 07:04
  • 2
    i'd say if your user can simply break it and cause a syntax error, then its broken. – pala_ Apr 21 '15 at 07:05
  • @user1456733 Could you provide the code that connects `$id` (or `$_GET['id']`) with the `select` function? Also, are users able to change *any* value in any table at all, probably by creating an account, writing a comment, or anything? And what permissions does your MySQL user have? – Siguza Apr 21 '15 at 07:09
  • @user1456733 you don't need to drop to be dangerous. `union` an unwanted table to a select and get information you should not be able to see from the database is a fair enough risk. – β.εηοιτ.βε Apr 21 '15 at 07:10
  • @b.enoit.be The most dangerous thing possible though would probably still be to get PHP code into the database and do a `SELECT ... INTO OUTFILE` somewhere inside the webroot :P – Siguza Apr 21 '15 at 07:12
  • 2
    The fact that you can make the query misbehave (err out) is a sign that an attacker has *some* leverage on the system. How much leverage exactly and what can be levered with it in practice is a different topic, but it certainly doesn't bode well at all. Pay a pentester for an hour and you'll know exactly how much leverage there is. – deceze Apr 21 '15 at 07:22
  • Or let [sqlmap](http://sqlmap.org/) on the loose :P – Siguza Apr 21 '15 at 07:23
  • @pala_ Indeed, good point. @Siguza I have just edited the question. As for your question: if `select` returns a user, that `$id` is also used to update the same table. After that, users are able to login and edit a single text field (other fields they have access to have preset values). – user1456733 Apr 21 '15 at 07:24
  • 1
    This might be illuminating: http://www.unixwiz.net/techtips/sql-injection.html – deceze Apr 21 '15 at 07:26
  • 1
    @user1456733 You can usually only exploit the queries of one type (select, update, ...) while breaking the other types. So whatever query comes first, that's most likely the only one exploitable. – Siguza Apr 21 '15 at 07:27
  • login would be more exploitable. does it use the same system? – pala_ Apr 21 '15 at 07:30
  • @deceze Excellent resource! One of the very first examples yielded success! `http://localhost/api/view.php?id=' or 'x' = 'x`. Would you (or anyone else) happen to know if I would be able to abuse this by using `UNION`, and how? – user1456733 Apr 21 '15 at 07:31
  • @pala_ Yes, it currently uses the same system. – user1456733 Apr 21 '15 at 07:34
  • 1
    @user1456733 That depends on how the result is printed. If you can select sensitive data from a table, but not print it, it is still of no use to you. But I really suggest you play around with sqlmap (see link in one of my previous comments). Also, are you allowed to and willing to upload a zip file with you PHP scripts together with a database dump so we can play around ourselves? – Siguza Apr 21 '15 at 07:49
  • Got it! Using the `' or 'x' = 'x` template I was successfully able to `UNION` into various other types of information. I learned quite a lot, so thank you everyone! If any of you wish to provide an answer (or answer wiki?), please use the above information and submit one. Thanks again! – user1456733 Apr 21 '15 at 07:50
  • @Siguza I assume not, but would it be possible to force the server to print the result, somehow? Also, I fear that I would not be allowed to provide either the dumps or the complete scripts, unfortunately, as there is some sensitive data contained. I do apologize for that :( – user1456733 Apr 21 '15 at 07:59
  • 1
    @user1456733 That would only be possible if you were able to execute code remotely, at which point it's game over anyway. But if, for example `$result['someColumn']` is printed, you could create a union that fetches some data from another table `... AS someColumn` and print that value(s) you need that way. – Siguza Apr 21 '15 at 08:06
  • @Siguza I see. Again, thank you (and everyone else who contributed) for the valuable information. – user1456733 Apr 21 '15 at 08:29
  • There are other techniques, such as discovering user names via a binary search and timing the replies. This is followed by checking for obvious passwords. – Rick James Apr 21 '15 at 18:35
  • @RickJames Thanks for the follow-up information! Could you provide any example or sources for those kinds of techniques? – user1456733 Apr 21 '15 at 21:23
  • You can do what I would do -- search the web. Your main protection against SQL Injection in PHP is to escape and quote anything that the user entered. The worst I have personally seen is a hacker that posted on the web a copy of /etc/passwd. I forget the details of how he could read the entire disk. – Rick James Apr 21 '15 at 21:56
  • I think it involved (1) sql injection, (2) GRANTing the app access to all databases (including `mysql`), (3) having a weak password for root, and (4) having local_infile = ON. We have been discussing #1. But here are 3 other holes to plug. – Rick James Apr 21 '15 at 21:59

1 Answers1

3

Just the fact alone that some specific input results in a MySQL syntax error is a proof that the user input has some unintended influence on the SQL syntax, which means SQL injection is possible.

But the exploitation of this SQL injection is a different chapter. And MySQL, or more precisely PHP’s MySQL extension, does not support the execution of multiple statements with mysql_query by default:

mysql_query() sends a unique query (multiple queries are not supported) […]

So the classic example of dropping a table won’t work and you’re limited to the capabilities of the current statement and the allowed syntax elements from the injection point on.

In your specific example, the SELECT limits you to

For reading arbitrary data, a UNION would be the best choice as the results seem to get reflected back to the user. All you have to ensure is that the resulting SQL is valid, which means your UNION must to have three columns (i. e., Replied, Response, and Registered from the existing SELECT):

' UNION SELECT '1','2','3

The resulting statement would then look like:

SELECT Replied, Response, Registered FROM invitations WHERE Id = '' UNION SELECT '1','2','3'

You now can replace the additionally selected values by your own expressions (including LOAD_FILE) or sub-queries.

Community
  • 1
  • 1
Gumbo
  • 643,351
  • 109
  • 780
  • 844