-2
public function fetchUserData( $username, $noUpdate = false ) {
  if ( DEBUG ) echo "DBInterface::fetchUserData( '$username' )\n";
  $query = "SELECT * FROM logins WHERE username = '$username'";
  $result = mysql_db_query( $this->database, $query, $this->dbc );
  if ( $result && !$noUpdate ) {
        mysql_db_query( $this->database, "UPDATE logins SET last_accessed = CURRENT_TIMESTAMP WHERE username = '$username' ", $this->dbc );
  }
  return $this->userData = mysql_fetch_assoc( $result );
}


public function verifyLogin( $username = null, $password = null ) {
  if ( DEBUG ) echo "DBInterface::verifyLogin( '$username', '$password' )\n";
  $success = ( $username && $password
            && $this->fetchUserData( $username )
            && $this->userData['password'] == $this->md5_base64( $password  )
            && $this->setLoggedIn()
          );
  return $success;
}

Obviously, there's no escape function, so one might insert as ' or '1'='1 to make WHERE clause true, and fetchUserData will return all rows from the table. But verfiyLogin checks user input password with the query result from database which may not be same, hence authentication will fail. Attacker also cannot modify table since mysql_db_query executes only single sql statement. Am I right? Any thoughts?

REALFREE
  • 4,378
  • 7
  • 40
  • 73
  • 2
    Even if it isn't possible in this case, why not use proper techniques? Read the big fat red warning: http://php.net/mysql_fetch_assoc – Daniel Sloof Feb 07 '14 at 06:47
  • use mysqli_* function to avoid sql injections – krishna Feb 07 '14 at 06:49
  • I was just wondering if it is possible to do sql injection here. This was from open source code, and obviously it is out of update. – REALFREE Feb 07 '14 at 06:51
  • 1
    As you have already seen, you can alter the query because the param is not escaped and that is already an injection. Even if it may be unpromlematic looking on it isolated, it could be a problem in combination with other code. So every query that can be altered that way should be handled as security problem. – t.niese Feb 07 '14 at 06:56
  • @krishna while `mysqli` or `PDO` should be used instead of the deprecated extension, they won't prevent from writing code that allow injection. – t.niese Feb 07 '14 at 07:22

3 Answers3

3

Yes, it's very possible to do SQL injection with any SQL query that's built from user input.

You should use the escaping functions, or preferentially prepared statements to protect yourself from SQL injection. However, you can't use prepared statements if you're using the outdated and deprecated mysql_* functions. You need to switch to mysqli or PDO.

PDO example:

$myPDO = new PDO ('Connection options go here');
$stmt = $myPDO -> prepare ("SELECT * FROM table WHERE rowID = :rowID");
if ($stmt -> execute (array ('rowID' = $inputFromUntrustedSource))) {
    while ($row = $stmt -> fetch ()) {
        // do stuff with $row here
    }
}
GordonM
  • 31,179
  • 15
  • 87
  • 129
2

You can inject your own data into the result set using the UNION operation. So an attacker could supply his own $this->userData['password'] value that would be equal to the $this->md5_base64($password) value, for example:

' UNION SELECT 'admin', 'X03MO1qnZdYdgyfeuILPmQ==

So you should absolutely make sure you pass the values properly to your query.

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

Data validation is often confused with SQL formatting. This "escaping" you are talking about (whatever you mean under this vague term) belongs not to "injection" but to mere formatting. You need to escape strings not because of injections but because of string delimiters and some other reasons.

Data validation rules may change. SQL formatting rules are constant. You have to format any data you put in SQL string. Whatever you did to this data prior constructing the query - doesn't matter.

Your Common Sense
  • 156,878
  • 40
  • 214
  • 345