2

I've written a simple PHP application (MyApp) that allows users to log into their Evernote account using Evernote's SDK for PHP. However, in my application, I want to do this:

  1. User approves Evernote login
  2. MyApp receives #1's requestToken, requestTokenSecret, etc.
  3. MyApp stores #2 into a cookie and $_SESSION
  4. When users later return to my site, MyApp logs them in via the cookie

Yes, I know that such cookie-based logins are insecure. My application however, stores no sensitive data. What is the simplest way to proceed? Here is my attempt:

//if cookie
if (isset($_COOKIE['requestToken'])) {
    $m = new mysqli(MYSQL_SERVER, MYSQL_USER, MYSQL_PASS, MYSQL_DB);
    $q = "SELECT uid FROM users WHERE token={$_COOKIE['requestToken']} and secret={$_COOKIE['requestTokenSecret']}";

   // cookie matches db?
   if ($result = $m->query($q)) { 
      $_SESSION['login']=TRUE;
      $result->close();
}
   else // no match
      $_SESSION['login']=FALSE; 
}
else { //no cookie - create
    if (EverNoteLogin()) {
       $year=time()+86400*365; //86400 = 1 day
       setcookie('requestToken',$_SESSION['requestToken'],$year); 
       setcookie('requestTokenSecret',$_SESSION['requestTokenSecret'],$year);
    }
}

SUGGESTED IMPROVEMENTS VERSION

//if cookie
if (isset($_COOKIE['requestToken'])) {
    //check db
    $m = new mysqli(MYSQL_SERVER, MYSQL_USER, MYSQL_PASS, MYSQL_DB);
    $s=$m->prepare("SELECT uid FROM user WHERE token=? AND secret=?");
    $s->bind_param('ss',$rt, $rs);
    $rt=$_COOKIE['requestToken'];
    $rs=$_COOKIE['requestTokenSecret'];
    $s->execute();
    $s->store_result(); 
    $n=$s->num_rows;
    //match db?
    if ($n) {
        if ($n==1) { // 1 match
                $s->bind_result($uid);
                $_SESSION['uid']=$uid;
                $_SESSION['login']=TRUE;
            }
        else
            $_SESSION['login']=FALSE; // >1 match not good
    }else
        $_SESSION['login']=FALSE;
}
else { //no cookie - create
    if (EverNoteLogin()) {
       $year=time()+86400*365; //86400 = 1 day
       setcookie('requestToken',$_SESSION['requestToken'],$year); 
       setcookie('requestTokenSecret',$_SESSION['requestTokenSecret'],$year);
    }
}
mellow-yellow
  • 1,670
  • 1
  • 18
  • 38
  • 3
    This is vulnerable to SQL injection. `$_COOKIE` is user data and cannot be trusted. Since you are using MySQLi, this ought to be a prepared statement rather than a `query()` call. – Michael Berkowski Nov 02 '12 at 01:20
  • First store the $_COOKIE info in variables and then escape them, ie. $token = mysql_real_escape_string($_COOKIE['requestToken']); – Richi González Nov 02 '12 at 03:31
  • 1
    Juapo2, from Mysql website (http://php.net/manual/en/function.mysql-real-escape-string.php): "Use of this extension is discouraged. Instead, the MySQLi or PDO_MySQL extension should be used." – mellow-yellow Nov 02 '12 at 15:37

2 Answers2

0

I think the version of your script which uses the prepared statement looks suitable from an SQL injection standpoint.

I would also certainly recommend that you set the cookie as a secure cookie (thus requiring https connection), and as an http only cookie (thus preventing access to javascript, etc.).

Mike Brant
  • 70,514
  • 10
  • 99
  • 103
0

Using bind_param is an effective defense against SQL injection in the case you show. Now it doesn't matter what the values in the cookie are, they can't change the semantics of your SQL statement.

Parameter placeholders in an SQL query always function as a single scalar value. If the value happens to contain SQL syntax, it doesn't matter, it'll work as if you had put a quoted string into your query with every special character escaped perfectly. Totally safe for the purpose of avoiding the SQL injection vulnerability.

You can read more about SQL injection in my presentation SQL Injection Myths and Fallacies, or in my book, SQL Antipatterns Volume 1: Avoiding the Pitfalls of Database Programming.

What is not safe, however, is storing authentication credentials in plaintext in a cookie. Don't do that.
See also Is it secure to store passwords in cookies? or How to store cookies containing sensitive data securely in PHP?

Bill Karwin
  • 538,548
  • 86
  • 673
  • 828
  • Agreed. Unless I'm mistaken though, the oAuth requestToken (in the cookie) is "temporary" by definition and useless without the oauthVerifier, which is not recorded in any cookies. Thus, the cookie data is essentially random already. – mellow-yellow Nov 02 '12 at 17:19
  • Then why do you set a 1 year expiration for a temporary token? ;-) – Bill Karwin Nov 02 '12 at 17:47