1

I want to know, if in my login form there is any SQL injection possible. If there is, what could the exploit's web form entry look like? I send username and password by html form (POST). The login function code is:


$username = $_POST['user'];
$password = $_POST['pwd'];
$password = md5(password);

$sql = "SELECT id_user, name FROM 'TAB_USER' WHERE user = '$username' AND passwd = '$password' AND enable='Y'";

$result = mysql_query($sql,$dbCon);
$check = mysql_num_rows($result);
mysql_free_result($result);

if ($check ==  '1') {
  $result = mysql_query($sql,$dbCon);
  $array = mysql_fetch_array($result);

  $id = $array['id_user'];
  $name = $array['name'];

  $_SESSION['SES_id_user']= $id;
  $_SESSION['SES_name']= $name;

  return $id;
}
else return false;

Xander
  • 35,616
  • 27
  • 114
  • 141
stefano
  • 19
  • 1
  • 1
  • 2
  • Read up on Perl's concept of taint. It is the best example and documentation of taint & injection I've seen. Even though it is for Perl, it applies to PHP: http://perldoc.perl.org/perlsec.html – Chloe Dec 19 '13 at 23:43

4 Answers4

15

Short answer: It's damn vulnerable.

Why? You are concatenating the values given by POST which come directly from what the user typed. Therefore, it is very simple to manipulate your query. Also, you are using mysql extension which is deprecated. You should be using mysqli or PDO to create prepared statements to protect against injection.

Here's a question from Stackoverflow that explains the matter in depth and how to solve it.

On a side note, you should not be using MD5 to encrypt passwords as it is a broken algorithm. Consider using bcrypt or PBKDF2 as explained here.

Simon
  • 3,202
  • 4
  • 26
  • 38
  • 6
    Note though that the "brokenness" of MD5 for password hashing is NOT the bit about MD5 collisions; collisions are irrelevant to password hashing. Problems with MD5 are that it is way too fast, and also unsalted. – Thomas Pornin Sep 13 '13 at 15:25
10

You write this:

$sql = "SELECT id_user, name FROM 'TAB_USER' WHERE user = '$username'
                                                           ^^^^^^^^^
                                                    SQL injection is here

The bad guy will send as "username" something which will, after replacement, make your SQL command look like this:

SELECT id_user, name FROM 'TAB_USER' WHERE user = 'Admin'; -- (some stuff here)

i.e. the attacker will set the "username" to the string: Admin'; --

See the nifty quote character ? That's the lever for the attack. Since your code brutally replaces $username in the SQL command, with "whatever the client provided", the client can provide a closing quote character, which terminates the string constant, then a semicolon, which terminates the command, then a -- which tells to the database "everything else on the line is a comment, just ignore it". And ignore it the database does !

This allows the attacker to log in as admin regardless of whatever password he used. This is textbook SQL injection. Please take note: you may be tempted to conclude that the problems come from the "quote" character, and simply filtering out would be sufficient to avoid the attack. This is not so. There are several (many) ways to abuse a brutal replacement such as the one you do, and it is very hard to catch them all; like Pokemons, each new version of the SQL database software introduces new species. People tried to think of SQL injection as an "escape troublesome characters" issue; so they designed this function. Then they tried again. And it still failed. Only sorrow awaits you on this path; don't walk it. Don't wait for a mysql_really_escape_string_this_time_i_said_please(). Instead, use prepared statements.

Thomas Pornin
  • 322,884
  • 58
  • 787
  • 955
4

Just user input is vulnerable and pwd is safe .

Examples of your query SQL injection pattern (they must use to user input).

'or'1'='1 and enable='Y'--
user'or 1=1--
'or 1=1--

An attacker can't inject with pwd parameter becuase it's hexed MD5, and hexed MD5 output pattern is ([a-fA-F\d]{32}) and it's not controllable for query injecting .

Additionally,if totally of your cookies works with SES_id_user and SES_name for checking users login, your mechanism is not safe !

-------

Update :

Attackers can't run stacked queries like DROP and ETC, becuase PHP mysql_query method does not support multiple (stacked) queries.

Sajjad Pourali
  • 944
  • 1
  • 10
  • 22
3

This would be vulnerable to even basic automated attacks. An input such as "'; Drop Table 'TAB_USER'; SELECT id_user, name FROM 'TAB_USER' WHERE user = 'someuser" (exclusive of the "s) would make three commands execute from your script. The first would be a valid selection, the second would drop your table entirely and the third would perform another selection. (ok, the third query would fail since the table is now gone, but it gives the general idea).

You need to validate any user input before it goes to a query and/or use parametrized SQL. For a username for example, it should really be alphanumeric without punctuation, which would render it safe against injection. Parametrized SQL is a nice secondary line of defense since the SQL statement understands it is a user value and should not contain commands.

As others have mentioned, the password storage mechanism is also woefully insecure as it is not being hashed and is a fast algorithm, so large rainbow tables exist which could be used to look up a plain text value that will produce whatever the given hash is in the DB. These combine to make it trivial to not only abuse your database but also to log in to another user without having to alter the DB at all (by selecting out the HASH to the session perhaps and dumping it later and then looking up the password in an existing rainbow table for MD5.

AJ Henderson
  • 41,896
  • 5
  • 63
  • 110