Use timing safe comparison to validate `state` parameter for social login
authorNiklas (Krymonota) <Krymonota@users.noreply.github.com>
Wed, 12 Aug 2020 13:59:26 +0000 (15:59 +0200)
committerNiklas (Krymonota) <Krymonota@users.noreply.github.com>
Wed, 12 Aug 2020 13:59:26 +0000 (15:59 +0200)
The Twitter social login is left out because the implementation still uses OAuth 1.0, which does not support the `state` parameter.

Closes #3501

wcfsetup/install/files/lib/action/FacebookAuthAction.class.php
wcfsetup/install/files/lib/action/GithubAuthAction.class.php
wcfsetup/install/files/lib/action/GoogleAuthAction.class.php

index 4060862bce4f2057cb79e3ee7733245198f6da81..ab50f7e87ff566cc044394057ce5259ef4644dcd 100644 (file)
@@ -68,7 +68,7 @@ class FacebookAuthAction extends AbstractAction {
                        }
                        
                        // validate state, validation of state is executed after fetching the access_token to invalidate 'code'
-                       if (!isset($_GET['state']) || $_GET['state'] != WCF::getSession()->getVar('__facebookInit')) throw new IllegalLinkException();
+                       if (!isset($_GET['state']) || !\hash_equals(WCF::getSession()->getVar('__facebookInit'), $_GET['state'])) throw new IllegalLinkException();
                        WCF::getSession()->unregister('__facebookInit');
                        
                        try {
index 8192670f6d52d919977aee1d3c625e899e5acbc9..db95872029a6e67c720ed4e524a6812dacc56310 100644 (file)
@@ -64,7 +64,7 @@ class GithubAuthAction extends AbstractAction {
                        }
                        
                        // validate state, validation of state is executed after fetching the access_token to invalidate 'code'
-                       if (!isset($_GET['state']) || $_GET['state'] != WCF::getSession()->getVar('__githubInit')) throw new IllegalLinkException();
+                       if (!isset($_GET['state']) || !\hash_equals(WCF::getSession()->getVar('__githubInit'), $_GET['state'])) throw new IllegalLinkException();
                        WCF::getSession()->unregister('__githubInit');
                        
                        parse_str($content, $data);
index 8211988ed49e386758e69c9d6da2a3aad92a6445..d3f349afa86bed3ff8f334ef3e1206df44057f49 100644 (file)
@@ -67,7 +67,7 @@ class GoogleAuthAction extends AbstractAction {
                        }
                        
                        // validate state, validation of state is executed after fetching the access_token to invalidate 'code'
-                       if (!isset($_GET['state']) || $_GET['state'] != WCF::getSession()->getVar('__googleInit')) throw new IllegalLinkException();
+                       if (!isset($_GET['state']) || !\hash_equals(WCF::getSession()->getVar('__googleInit'), $_GET['state'])) throw new IllegalLinkException();
                        WCF::getSession()->unregister('__googleInit');
                        
                        $data = JSON::decode($content);