CVE-2014-9459: Cross-Site Request Forgery (CSRF) (CWE-352)
value: Cross-site request forgery (CSRF) vulnerability in the AdminObserver function in e107_admin/users.php in e107 2.0 alpha2 allows remote attackers to hijack the authentication of administrators for requests that add users to the administrator group via the id parameter in an admin action.
CWE description: The web application does not, or can not, sufficiently verify whether a well-formed, valid, consistent request was intentionally provided by the user who submitted the request.
Published: 2015-01-02T20:59Z
Repository: https://github.com/e107inc/e107
Language: PHP
Files changed: e107_admin/users.php, e107_handlers/user_handler.php
Commit date: 2014-12-27 12:02:12 -0800
Commit message: Added additional check prior to administrator status change.

Diff: diff --git a/e107_admin/users.php b/e107_admin/users.php
index 25413f1e0..2c99909c7 100644
--- a/e107_admin/users.php
+++ b/e107_admin/users.php
@@ -693,19 +693,27 @@ class users_admin_ui extends e_admin_ui
 			// TODO lan
 			$mes->addError("User not found.", 'default', true);
 			$this->redirect('list', 'main', true);
 		}
 		
-		if(!$sysuser->isAdmin())
+	
+		if($this->getPosted('update_admin'))
+		{
+			 e107::getUserPerms()->updatePerms($userid, $_POST['perms']);
+			 $this->redirect('list', 'main', true);
+		}
+		
+		if(!$sysuser->isAdmin()) // Security Check Only. Admin status check is added during 'updatePerms'. 
 		{
-			$sysuser->set('user_admin', 1)->save(); //"user","user_admin='1' WHERE user_id={$userid}"
-			$lan = str_replace(array('--UID--', '--NAME--', '--EMAIL--'), array($sysuser->getId(), $sysuser->getName(), $sysuser->getValue('email')), USRLAN_164);
-			e107::getLog()->add('USET_08', $lan, E_LOG_INFORMATIVE);
-			$mes->addSuccess($lan);
+		//	$sysuser->set('user_admin', 1)->save(); //"user","user_admin='1' WHERE user_id={$userid}"
+		//	$lan = str_replace(array('--UID--', '--NAME--', '--EMAIL--'), array($sysuser->getId(), $sysuser->getName(), $sysuser->getValue('email')), USRLAN_164);
+		//	e107::getLog()->add('USET_08', $lan, E_LOG_INFORMATIVE);
+		//	$mes->addSuccess($lan);
+			$mes->addWarning("You are about to make User #<b>".$sysuser->getId()."</b> : <b>".$sysuser->getName()."</b> (".$sysuser->getValue('email').") an <b>administrator</b>."); ///TODO LAN
+			$mes->addWarning("Set the permissions and click <b>Update</b> to proceed or <b>Back</b> to abort.");
 		}
 		
-		if($this->getPosted('update_admin')) e107::getUserPerms()->updatePerms($userid, $_POST['perms']);
 	}
 	
 	/**
 	 * Admin manage page
 	 */
diff --git a/e107_handlers/user_handler.php b/e107_handlers/user_handler.php
index 0b2c7eb05..1c156a5f5 100644
--- a/e107_handlers/user_handler.php
+++ b/e107_handlers/user_handler.php
@@ -1596,10 +1596,17 @@ class e_userperms
 				$perm .= $value.".";
 			}
 	 	}
 		
 		//$sql->db_Update("user", "user_perms='{$perm}' WHERE user_id='{$modID}' ") 
+		if(!$sysuser->isAdmin())
+		{
+			$sysuser->set('user_admin', 1)->save();
+			$lan = str_replace(array('--UID--', '--NAME--', '--EMAIL--'), array($sysuser->getId(), $sysuser->getName(), $sysuser->getValue('email')), USRLAN_164);
+			e107::getLog()->add('USET_08', $lan, E_LOG_INFORMATIVE);
+		}
+		
 		e107::getMessage()->addAuto($sysuser->set('user_perms', $perm)->save(), 'update', sprintf(LAN_UPDATED, $tp->toDB($_POST['ad_name'])), false, false);
 		$logMsg = str_replace(array('--ID--', '--NAME--'),array($modID, $a_name),ADMSLAN_72).$perm;
 		e107::getLog()->add('ADMIN_01',$logMsg,E_LOG_INFORMATIVE,'');
 	}
 

Vulnerable code: 		if(!$sysuser->isAdmin())
			$sysuser->set('user_admin', 1)->save(); //"user","user_admin='1' WHERE user_id={$userid}"
			$lan = str_replace(array('--UID--', '--NAME--', '--EMAIL--'), array($sysuser->getId(), $sysuser->getName(), $sysuser->getValue('email')), USRLAN_164);
			e107::getLog()->add('USET_08', $lan, E_LOG_INFORMATIVE);
			$mes->addSuccess($lan);
		if($this->getPosted('update_admin')) e107::getUserPerms()->updatePerms($userid, $_POST['perms']);
Fixed code: 	
		if($this->getPosted('update_admin'))
		{
			 e107::getUserPerms()->updatePerms($userid, $_POST['perms']);
			 $this->redirect('list', 'main', true);
		}
		
		if(!$sysuser->isAdmin()) // Security Check Only. Admin status check is added during 'updatePerms'. 
		//	$sysuser->set('user_admin', 1)->save(); //"user","user_admin='1' WHERE user_id={$userid}"
		//	$lan = str_replace(array('--UID--', '--NAME--', '--EMAIL--'), array($sysuser->getId(), $sysuser->getName(), $sysuser->getValue('email')), USRLAN_164);
		//	e107::getLog()->add('USET_08', $lan, E_LOG_INFORMATIVE);
		//	$mes->addSuccess($lan);
			$mes->addWarning("You are about to make User #<b>".$sysuser->getId()."</b> : <b>".$sysuser->getName()."</b> (".$sysuser->getValue('email').") an <b>administrator</b>."); ///TODO LAN
			$mes->addWarning("Set the permissions and click <b>Update</b> to proceed or <b>Back</b> to abort.");
		if(!$sysuser->isAdmin())
		{
			$sysuser->set('user_admin', 1)->save();
			$lan = str_replace(array('--UID--', '--NAME--', '--EMAIL--'), array($sysuser->getId(), $sysuser->getName(), $sysuser->getValue('email')), USRLAN_164);
			e107::getLog()->add('USET_08', $lan, E_LOG_INFORMATIVE);
		}