Fix Anti-CSRF token disclosure in online status location

mybb · Jul 17, 2020 · 1b25406 · 1b25406
1 parent 548b694
commit 1b25406
Show file tree

Hide file tree

Showing 2 changed files with 53 additions and 30 deletions.
diff --git a/inc/class_session.php b/inc/class_session.php
@@ -35,12 +35,22 @@ class session
 	 */
 	public $is_spider = false;
 
+	/**
+	 * Request parameters that are to be ignored for location storage
+	 *
+	 * @var array
+	 */
+	public $ignore_parameters = array(
+		'my_post_key',
+		'logoutkey',
+	);
+
 	/**
 	 * Initialize a session
 	 */
 	function init()
 	{
-		global $db, $mybb, $cache;
+		global $db, $mybb, $cache, $plugins;
 
 		// Get our visitor's IP.
 		$this->ipaddress = get_ip();
@@ -66,6 +76,11 @@ function init()
 			}
 		}
 
+		if(isset($plugins))
+		{
+			$plugins->run_hooks('pre_session_load', $this);
+		}
+
 		// If we have a valid session id and user id, load that users session.
 		if(!empty($mybb->cookies['mybbuser']))
 		{
@@ -478,8 +493,8 @@ function update_session($sid, $uid=0)
 			$onlinedata['uid'] = 0;
 		}
 		$onlinedata['time'] = TIME_NOW;
-		
-		$onlinedata['location'] = $db->escape_string(substr(get_current_location(), 0, 150));
+
+		$onlinedata['location'] = $db->escape_string(substr(get_current_location(false, $this->ignore_parameters), 0, 150));
 		$onlinedata['useragent'] = $db->escape_string(my_substr($this->useragent, 0, 200));
 
 		$onlinedata['location1'] = (int)$speciallocs['1'];
@@ -527,8 +542,8 @@ function create_session($uid=0)
 		}
 		$onlinedata['time'] = TIME_NOW;
 		$onlinedata['ip'] = $db->escape_binary($this->packedip);
-		
-		$onlinedata['location'] = $db->escape_string(substr(get_current_location(), 0, 150));
+
+		$onlinedata['location'] = $db->escape_string(substr(get_current_location(false, $this->ignore_parameters), 0, 150));
 		$onlinedata['useragent'] = $db->escape_string(my_substr($this->useragent, 0, 200));
 
 		$onlinedata['location1'] = (int)$speciallocs['1'];

diff --git a/inc/functions.php b/inc/functions.php
@@ -5189,12 +5189,14 @@ function leave_usergroup($uid, $leavegroup)
  * Get the current location taking in to account different web serves and systems
  *
  * @param boolean $fields True to return as "hidden" fields
- * @param array $ignore Array of fields to ignore if first argument is true
+ * @param array $ignore Array of fields to ignore for returning "hidden" fields or URL being accessed
  * @param boolean $quick True to skip all inputs and return only the file path part of the URL
- * @return string The current URL being accessed
+ * @return string|array The current URL being accessed or form data if $fields is true
  */
 function get_current_location($fields=false, $ignore=array(), $quick=false)
 {
+	global $mybb;
+
 	if(defined("MYBB_LOCATION"))
 	{
 		return MYBB_LOCATION;
@@ -5226,14 +5228,13 @@ function get_current_location($fields=false, $ignore=array(), $quick=false)
 		return $location;
 	}
 
-	if($fields == true)
+	if(!is_array($ignore))
 	{
-		global $mybb;
+		$ignore = array($ignore);
+	}
 
-		if(!is_array($ignore))
-		{
-			$ignore = array($ignore);
-		}
+	if($fields == true)
+	{
 
 		$form_html = '';
 		if(!empty($mybb->input))
@@ -5253,39 +5254,46 @@ function get_current_location($fields=false, $ignore=array(), $quick=false)
 	}
 	else
 	{
+		$parameters = array();
+
 		if(isset($_SERVER['QUERY_STRING']))
 		{
-			$location .= "?".htmlspecialchars_uni($_SERVER['QUERY_STRING']);
+			$current_query_string = $_SERVER['QUERY_STRING'];
 		}
 		else if(isset($_ENV['QUERY_STRING']))
 		{
-			$location .= "?".htmlspecialchars_uni($_ENV['QUERY_STRING']);
+			$current_query_string = $_ENV['QUERY_STRING'];
+		} else
+		{
+			$current_query_string = '';
+		}
+
+		parse_str($current_query_string, $current_parameters);
+
+		foreach($current_parameters as $name => $value)
+		{
+			if(!in_array($name, $ignore))
+			{
+				$parameters[$name] = $value;
+			}
 		}
 
-		if((isset($_SERVER['REQUEST_METHOD']) && $_SERVER['REQUEST_METHOD'] == "POST") || (isset($_ENV['REQUEST_METHOD']) && $_ENV['REQUEST_METHOD'] == "POST"))
+		if($mybb->request_method === 'post')
 		{
 			$post_array = array('action', 'fid', 'pid', 'tid', 'uid', 'eid');
 
 			foreach($post_array as $var)
 			{
-				if(isset($_POST[$var]))
+				if(isset($_POST[$var]) && !in_array($var, $ignore))
 				{
-					$addloc[] = urlencode($var).'='.urlencode($_POST[$var]);
+					$parameters[$var] = $_POST[$var];
 				}
 			}
+		}
 
-			if(isset($addloc) && is_array($addloc))
-			{
-				if(strpos($location, "?") === false)
-				{
-					$location .= "?";
-				}
-				else
-				{
-					$location .= "&amp;";
-				}
-				$location .= implode("&amp;", $addloc);
-			}
+		if(!empty($parameters))
+		{
+			$location .= '?'.http_build_query($parameters, '', '&amp;');
 		}
 
 		return $location;