Solve a security issue with some modules (not validating URLs we are...

Solve a security issue with some modules (not validating URLs we are redirecting to) by moving the check to the SimpleSAML_Auth_State::loadState() method.

lib/SimpleSAML/Auth/ProcessingChain.php

@@ -301,11 +301,9 @@ class SimpleSAML_Auth_ProcessingChain {
 	/**
 	 * Retrieve a state which has finished processing.
 	 *
-	 * @param string $id The state identifier. This can be found in the
-	 * SimpleSAML_Auth_ProcessingChain::AUTHPARAM request parameter. Please
-	 * make sure to sanitize it properly by calling the
-	 * SimpleSAML_Utilities::checkURLAllowed() function with the embedded
-	 * restart URL, if any. See also SimpleSAML_Utilities::parseStateID().
+	 * @param string $id The state identifier.
+     * @see SimpleSAML_Utilities::parseStateID()
+     * @return Array The state referenced by the $id parameter.
 	 */
 	public static function fetchProcessedState($id) {
 		assert('is_string($id)');

lib/SimpleSAML/Auth/State.php

@@ -225,7 +225,7 @@ class SimpleSAML_Auth_State {
 				throw new SimpleSAML_Error_NoState();
 			}
 
-			SimpleSAML_Utilities::redirectTrustedURL($sid['url']);
+			SimpleSAML_Utilities::redirectUntrustedURL($sid['url']);
 		}
 
 		$state = unserialize($state);
@@ -249,7 +249,7 @@ class SimpleSAML_Auth_State {
 				throw new Exception($msg);
 			}
 
-			SimpleSAML_Utilities::redirectTrustedURL($sid['url']);
+			SimpleSAML_Utilities::redirectUntrustedURL($sid['url']);
 		}
 
 		return $state;

lib/SimpleSAML/IdP/LogoutTraditional.php

@@ -75,12 +75,6 @@ class SimpleSAML_IdP_LogoutTraditional extends SimpleSAML_IdP_LogoutHandler {
 			throw new SimpleSAML_Error_Exception('RelayState lost during logout.');
 		}
 
-		// sanitize the input
-		$sid = SimpleSAML_Utilities::parseStateID($relayState);
-		if (!is_null($sid['url'])) {
-			SimpleSAML_Utilities::checkURLAllowed($sid['url']);
-		}
-
 		$state = SimpleSAML_Auth_State::loadState($relayState, 'core:LogoutTraditional');
 
 		if ($error === NULL) {

modules/aselect/www/credentials.php

@@ -9,15 +9,7 @@
 if (!array_key_exists('ssp_state', $_REQUEST)) {
     throw new SimpleSAML_Error_Exception("Missing ssp_state parameter");
 }
-$id = $_REQUEST['ssp_state'];
-
-// sanitize the input
-$sid = SimpleSAML_Utilities::parseStateID($id);
-if (!is_null($sid['url'])) {
-    SimpleSAML_Utilities::checkURLAllowed($sid['url']);
-}
-
-$state = SimpleSAML_Auth_State::loadState($id, 'aselect:login');
+$state = SimpleSAML_Auth_State::loadState($_REQUEST['ssp_state'], 'aselect:login');
 
 if (!array_key_exists('a-select-server', $_REQUEST)) {
     SimpleSAML_Auth_State::throwException($state, new SimpleSAML_Error_Exception("Missing a-select-server parameter"));

modules/authYubiKey/lib/Auth/Source/YubiKey.php

@@ -123,12 +123,6 @@ class sspmod_authYubiKey_Auth_Source_YubiKey extends SimpleSAML_Auth_Source {
 		assert('is_string($authStateId)');
 		assert('is_string($otp)');
 
-		// sanitize the input
-		$sid = SimpleSAML_Utilities::parseStateID($authStateId);
-		if (!is_null($sid['url'])) {
-			SimpleSAML_Utilities::checkURLAllowed($sid['url']);
-		}
-
 		/* Retrieve the authentication state. */
 		$state = SimpleSAML_Auth_State::loadState($authStateId, self::STAGEID);
 

modules/authfacebook/www/linkback.php

@@ -7,16 +7,7 @@
 if (!array_key_exists('AuthState', $_REQUEST) || empty($_REQUEST['AuthState'])) {
 	throw new SimpleSAML_Error_BadRequest('Missing state parameter on facebook linkback endpoint.');
 }
-
-$stateID = $_REQUEST['AuthState'];
-
-// sanitize the input
-$sid = SimpleSAML_Utilities::parseStateID($stateID);
-if (!is_null($sid['url'])) {
-	SimpleSAML_Utilities::checkURLAllowed($sid['url']);
-}
-
-$state = SimpleSAML_Auth_State::loadState($stateID, sspmod_authfacebook_Auth_Source_Facebook::STAGE_INIT);
+$state = SimpleSAML_Auth_State::loadState($_REQUEST['AuthState'], sspmod_authfacebook_Auth_Source_Facebook::STAGE_INIT);
 
 /* Find authentication source. */
 if (!array_key_exists(sspmod_authfacebook_Auth_Source_Facebook::AUTHID, $state)) {

modules/authlinkedin/www/linkback.php

@@ -4,19 +4,10 @@
  * Handle linkback() response from LinkedIn.
  */
 
-if (array_key_exists('stateid', $_REQUEST)) {
-        $stateId = $_REQUEST['stateid'];
-} else {
-        throw new Exception('Lost OAuth Client State');
-}
-
-// sanitize the input
-$sid = SimpleSAML_Utilities::parseStateID($stateId);
-if (!is_null($sid['url'])) {
-	SimpleSAML_Utilities::checkURLAllowed($sid['url']);
+if (!array_key_exists('stateid', $_REQUEST)) {
+    throw new Exception('Lost OAuth Client State');
 }
-
-$state = SimpleSAML_Auth_State::loadState($stateId, sspmod_authlinkedin_Auth_Source_LinkedIn::STAGE_INIT);
+$state = SimpleSAML_Auth_State::loadState($_REQUEST['stateid'], sspmod_authlinkedin_Auth_Source_LinkedIn::STAGE_INIT);
 
 // http://developer.linkedin.com/docs/DOC-1008#2_Redirect_the_User_to_our_Authorization_Server
 if (array_key_exists('oauth_verifier', $_REQUEST)) {

modules/authmyspace/www/linkback.php

@@ -4,19 +4,10 @@
  * Handle linkback() response from MySpace.
  */
 
-if (array_key_exists('stateid', $_REQUEST)) {
-	$stateId = $_REQUEST['stateid'];
-} else {
+if (!array_key_exists('stateid', $_REQUEST)) {
 	throw new Exception('State Lost - not returned by MySpace Auth');
 }
-
-// sanitize the input
-$sid = SimpleSAML_Utilities::parseStateID($stateId);
-if (!is_null($sid['url'])) {
-	SimpleSAML_Utilities::checkURLAllowed($sid['url']);
-}
-
-$state = SimpleSAML_Auth_State::loadState($stateId, sspmod_authmyspace_Auth_Source_MySpace::STAGE_INIT);
+$state = SimpleSAML_Auth_State::loadState($_REQUEST['stateid'], sspmod_authmyspace_Auth_Source_MySpace::STAGE_INIT);
 
 if (array_key_exists('oauth_problem', $_REQUEST)) {
 	// oauth_problem of 'user_refused' means user chose not to login with MySpace

modules/authorize/www/authorize_403.php

@@ -8,16 +8,7 @@
 if (!array_key_exists('StateId', $_REQUEST)) {
 	throw new SimpleSAML_Error_BadRequest('Missing required StateId query parameter.');
 }
-
-$id = $_REQUEST['StateId'];
-
-// sanitize the input
-$sid = SimpleSAML_Utilities::parseStateID($id);
-if (!is_null($sid['url'])) {
-	SimpleSAML_Utilities::checkURLAllowed($sid['url']);
-}
-
-$state = SimpleSAML_Auth_State::loadState($id, 'authorize:Authorize');
+$state = SimpleSAML_Auth_State::loadState($_REQUEST['StateId'], 'authorize:Authorize');
 
 $globalConfig = SimpleSAML_Configuration::getInstance();
 $t = new SimpleSAML_XHTML_Template($globalConfig, 'authorize:authorize_403.php');

modules/authtwitter/www/linkback.php

@@ -7,15 +7,7 @@
 if (!array_key_exists('AuthState', $_REQUEST) || empty($_REQUEST['AuthState'])) {
 	throw new SimpleSAML_Error_BadRequest('Missing state parameter on twitter linkback endpoint.');
 }
-$stateID = $_REQUEST['AuthState'];
-
-// sanitize the input
-$sid = SimpleSAML_Utilities::parseStateID($stateID);
-if (!is_null($sid['url'])) {
-	SimpleSAML_Utilities::checkURLAllowed($sid['url']);
-}
-
-$state = SimpleSAML_Auth_State::loadState($stateID, sspmod_authtwitter_Auth_Source_Twitter::STAGE_INIT);
+$state = SimpleSAML_Auth_State::loadState($_REQUEST['AuthState'], sspmod_authtwitter_Auth_Source_Twitter::STAGE_INIT);
 
 /* Find authentication source. */
 if (!array_key_exists(sspmod_authtwitter_Auth_Source_Twitter::AUTHID, $state)) {

modules/authwindowslive/www/linkback.php

@@ -4,19 +4,10 @@
  * Handle linkback() response from Windows Live ID.
  */
 
-if (array_key_exists('wrap_client_state', $_REQUEST)) {
-	$stateId = $_REQUEST['wrap_client_state'];
-	
-	// sanitize the input
-	$sid = SimpleSAML_Utilities::parseStateID($stateId);
-	if (!is_null($sid['url'])) {
-		SimpleSAML_Utilities::checkURLAllowed($sid['url']);
-	}
-
-	$state = SimpleSAML_Auth_State::loadState($stateId, sspmod_authwindowslive_Auth_Source_LiveID::STAGE_INIT);
-} else {
+if (!array_key_exists('wrap_client_state', $_REQUEST)) {
 	throw new Exception('Lost OAuth-WRAP Client State');
 }
+$state = SimpleSAML_Auth_State::loadState($_REQUEST['wrap_client_state'], sspmod_authwindowslive_Auth_Source_LiveID::STAGE_INIT);
 
 // http://msdn.microsoft.com/en-us/library/ff749771.aspx
 if (array_key_exists('wrap_verification_code', $_REQUEST)) {

modules/cas/www/linkback.php

@@ -7,19 +7,11 @@
 if (!isset($_GET['stateID'])) {
 	throw new SimpleSAML_Error_BadRequest('Missing stateID parameter.');
 }
-$stateId = (string)$_GET['stateID'];
+$state = SimpleSAML_Auth_State::loadState($_GET['stateID'], sspmod_cas_Auth_Source_CAS::STAGE_INIT);
 
 if (!isset($_GET['ticket'])) {
 	throw new SimpleSAML_Error_BadRequest('Missing ticket parameter.');
 }
-
-// sanitize the input
-$sid = SimpleSAML_Utilities::parseStateID($stateId);
-if (!is_null($sid['url'])) {
-	SimpleSAML_Utilities::checkURLAllowed($sid['url']);
-}
-
-$state = SimpleSAML_Auth_State::loadState($stateId, sspmod_cas_Auth_Source_CAS::STAGE_INIT);
 $state['cas:ticket'] = (string)$_GET['ticket'];
 
 /* Find authentication source. */

modules/cdc/www/resume.php

@@ -16,13 +16,6 @@ if ($response === NULL) {
 if (!isset($response['id'])) {
 	throw new SimpleSAML_Error_BadRequest('CDCResponse without id.');
 }
-
-// sanitize the input
-$sid = SimpleSAML_Utilities::parseStateID($response['id']);
-if (!is_null($sid['url'])) {
-	SimpleSAML_Utilities::checkURLAllowed($sid['url']);
-}
-
 $state = SimpleSAML_Auth_State::loadState($response['id'], 'cdc:resume');
 
 SimpleSAML_Auth_ProcessingChain::resumeProcessing($state);

modules/consent/www/getconsent.php

@@ -30,13 +30,6 @@ if (!array_key_exists('StateId', $_REQUEST)) {
 }
 
 $id = $_REQUEST['StateId'];
-
-// sanitize the input
-$sid = SimpleSAML_Utilities::parseStateID($id);
-if (!is_null($sid['url'])) {
-	SimpleSAML_Utilities::checkURLAllowed($sid['url']);
-}
-
 $state = SimpleSAML_Auth_State::loadState($id, 'consent:request');
 
 if (array_key_exists('core:SP', $state)) {

modules/consent/www/logout.php

@@ -8,15 +8,7 @@
 if (!array_key_exists('StateId', $_GET)) {
     throw new SimpleSAML_Error_BadRequest('Missing required StateId query parameter.');
 }
-$id = (string)$_GET['StateId'];
-
-// sanitize the input
-$sid = SimpleSAML_Utilities::parseStateID($id);
-if (!is_null($sid['url'])) {
-	SimpleSAML_Utilities::checkURLAllowed($sid['url']);
-}
-
-$state = SimpleSAML_Auth_State::loadState($id, 'consent:request');
+$state = SimpleSAML_Auth_State::loadState($_GET['StateId'], 'consent:request');
 
 $state['Responder'] = array('sspmod_consent_Logout', 'postLogout');
 

modules/consent/www/noconsent.php

@@ -11,13 +11,6 @@ if (!array_key_exists('StateId', $_REQUEST)) {
 }
 
 $id = $_REQUEST['StateId'];
-
-// sanitize the input
-$sid = SimpleSAML_Utilities::parseStateID($id);
-if (!is_null($sid['url'])) {
-	SimpleSAML_Utilities::checkURLAllowed($sid['url']);
-}
-
 $state = SimpleSAML_Auth_State::loadState($id, 'consent:request');
 
 $resumeFrom = SimpleSAML_Module::getModuleURL(

modules/core/lib/Auth/UserPassBase.php

@@ -233,12 +233,6 @@ abstract class sspmod_core_Auth_UserPassBase extends SimpleSAML_Auth_Source {
 		assert('is_string($username)');
 		assert('is_string($password)');
 
-		// sanitize the input
-		$sid = SimpleSAML_Utilities::parseStateID($authStateId);
-		if (!is_null($sid['url'])) {
-			SimpleSAML_Utilities::checkURLAllowed($sid['url']);
-		}
-
 		/* Here we retrieve the state array we saved in the authenticate-function. */
 		$state = SimpleSAML_Auth_State::loadState($authStateId, self::STAGEID);
 

modules/core/lib/Auth/UserPassOrgBase.php

@@ -208,12 +208,6 @@ abstract class sspmod_core_Auth_UserPassOrgBase extends SimpleSAML_Auth_Source {
 		assert('is_string($password)');
 		assert('is_string($organization)');
 
-		// sanitize the input
-		$sid = SimpleSAML_Utilities::parseStateID($authStateId);
-		if (!is_null($sid['url'])) {
-			SimpleSAML_Utilities::checkURLAllowed($sid['url']);
-		}
-
 		/* Retrieve the authentication state. */
 		$state = SimpleSAML_Auth_State::loadState($authStateId, self::STAGEID);
 
@@ -262,12 +256,6 @@ abstract class sspmod_core_Auth_UserPassOrgBase extends SimpleSAML_Auth_Source {
 	public static function listOrganizations($authStateId) {
 		assert('is_string($authStateId)');
 
-		// sanitize the input
-		$sid = SimpleSAML_Utilities::parseStateID($authStateId);
-		if (!is_null($sid['url'])) {
-			SimpleSAML_Utilities::checkURLAllowed($sid['url']);
-		}
-
 		/* Retrieve the authentication state. */
 		$state = SimpleSAML_Auth_State::loadState($authStateId, self::STAGEID);
 

modules/core/www/idp/logout-iframe-done.php

@@ -3,15 +3,7 @@
 if (!isset($_REQUEST['id'])) {
 	throw new SimpleSAML_Error_BadRequest('Missing required parameter: id');
 }
-$id = (string)$_REQUEST['id'];
-
-// sanitize the input
-$sid = SimpleSAML_Utilities::parseStateID($id);
-if (!is_null($sid['url'])) {
-	SimpleSAML_Utilities::checkURLAllowed($sid['url']);
-}
-
-$state = SimpleSAML_Auth_State::loadState($id, 'core:Logout-IFrame');
+$state = SimpleSAML_Auth_State::loadState($_REQUEST['id'], 'core:Logout-IFrame');
 $idp = SimpleSAML_IdP::getByState($state);
 
 $associations = $idp->getAssociations();

modules/core/www/idp/logout-iframe.php

@@ -3,7 +3,6 @@
 if (!isset($_REQUEST['id'])) {
 	throw new SimpleSAML_Error_BadRequest('Missing required parameter: id');
 }
-$id = (string)$_REQUEST['id'];
 
 if (isset($_REQUEST['type'])) {
 	$type = (string)$_REQUEST['type'];
@@ -19,13 +18,7 @@ if ($type !== 'embed' && $type !== 'async') {
 	SimpleSAML_Stats::log('core:idp:logout-iframe:page', array('type' => $type));
 }
 
-// sanitize the input
-$sid = SimpleSAML_Utilities::parseStateID($id);
-if (!is_null($sid['url'])) {
-	SimpleSAML_Utilities::checkURLAllowed($sid['url']);
-}
-
-$state = SimpleSAML_Auth_State::loadState($id, 'core:Logout-IFrame');
+$state = SimpleSAML_Auth_State::loadState($_REQUEST['id'], 'core:Logout-IFrame');
 $idp = SimpleSAML_IdP::getByState($state);
 
 if ($type !== 'init') {