0

I believe this statement is vulnerable for injection, but am unsure.

Any help in this regard?

Thanks.

function CheckUserLogin($userName,$password)
{
    $sql="SELECT user_id 
         FROM users 
         WHERE user_name='".addslashes($userName)."' AND password ='".addslashes($password)."'";    
    $this->query($sql);
    if($this->movenext()>0)
    {
        return $this->col['user_id'];   
    }
    else 
    {
        return false;
    }
}

The query function is as such

function query($_query){
    list($usec, $sec) = explode(" ",microtime());
    $time_start  = ((float)$usec + (float)$sec);


    $this->query = $_query;
    $this->result = @mysql_query($_query, $this->link_id) or die("<b>error in sql query</b><br><pre>".$_query."</pre>.mysql error : <b>".mysql_error($this->link_id)."</b><p>");

    list($usec, $sec) = explode(" ",microtime());
    $time_end  =  ((float)$usec + (float)$sec);
    $time = $time_end - $time_start;
}
Hamza Zafeer
  • 2,360
  • 13
  • 30
  • 42
Zac Jacob
  • 72
  • 2
  • 7

2 Answers2

1

This is what the docs on addslashes say:

Please note that use of addslashes() for database parameter escaping can be cause of security issues on most databases.

The same article has:

To escape database parameters, DBMS specific escape function (e.g. mysqli_real_escape_string() for MySQL or pg_escape_literal(), pg_escape_string() for PostgreSQL) should be used for security reasons.

However, the best way to use parameters in SQL statements is by using prepared statements. There the parameter values are never injected into the SQL statement, and so there cannot be malicious injection either. Instead the database engine uses the parameter values directly (and as literals) to execute the already compiled query.

Please read How can I prevent SQL injection in PHP: it has lots of information about this subject.

Community
  • 1
  • 1
trincot
  • 317,000
  • 35
  • 244
  • 286