From d6a5c93e0128c44c5382970c2a9f07db3fc0b0ae Mon Sep 17 00:00:00 2001 From: Ralf Becker Date: Thu, 26 Jun 2014 17:38:29 +0000 Subject: [PATCH] new php_safe_unserialize function refusing to unserialize objects and using it for config, preferences and DB-backups --- phpgwapi/inc/class.config.inc.php | 4 +- phpgwapi/inc/class.db_backup.inc.php | 12 +++++- phpgwapi/inc/class.preferences.inc.php | 6 +-- phpgwapi/inc/common_functions.inc.php | 55 ++++++++++++++++++++++++++ 4 files changed, 70 insertions(+), 7 deletions(-) diff --git a/phpgwapi/inc/class.config.inc.php b/phpgwapi/inc/class.config.inc.php index 5a50c701f3..a838a8db55 100755 --- a/phpgwapi/inc/class.config.inc.php +++ b/phpgwapi/inc/class.config.inc.php @@ -314,11 +314,11 @@ class config return $str; } // handling of old PHP serialized and addslashed prefs - $data = unserialize($str); + $data = php_safe_unserialize($str); if($data === false) { // manually retrieve the string lengths of the serialized array if unserialize failed - $data = unserialize(preg_replace_callback('!s:(\d+):"(.*?)";!s', function($matches) + $data = php_safe_unserialize(preg_replace_callback('!s:(\d+):"(.*?)";!s', function($matches) { return 's:'.mb_strlen($matches[2],'8bit').':"'.$matches[2].'";'; }, $str)); diff --git a/phpgwapi/inc/class.db_backup.inc.php b/phpgwapi/inc/class.db_backup.inc.php index 0821ae79c1..900bc0ffb3 100644 --- a/phpgwapi/inc/class.db_backup.inc.php +++ b/phpgwapi/inc/class.db_backup.inc.php @@ -449,7 +449,15 @@ class db_backup if (substr($line,0,8) == 'schema: ') { // create the tables in the backup set - $this->schemas = unserialize(trim(substr($line,8))); + $schema = trim(substr($line,8)); + if ($schema[0] == 'a' && $schema[1] == ':') + { + $this->schemas = php_safe_unserialize($schema); + } + else + { + $this->schema = json_decode($schema, true); + } foreach($this->schemas as $table_name => $schema) { // if column is longtext in current schema, convert text to longtext, in case user already updated column @@ -825,7 +833,7 @@ class db_backup $this->schema_backup($f); // add the schema in a human readable form too - fwrite($f,"\nschema: ".serialize($this->schemas)."\n"); + fwrite($f,"\nschema: ".json_encode($this->schemas)."\n"); foreach($this->schemas as $table => $schema) { diff --git a/phpgwapi/inc/class.preferences.inc.php b/phpgwapi/inc/class.preferences.inc.php index 6a760a9f08..58b9453c3f 100644 --- a/phpgwapi/inc/class.preferences.inc.php +++ b/phpgwapi/inc/class.preferences.inc.php @@ -349,16 +349,16 @@ class preferences protected static function unserialize($str) { // handling of new json-encoded prefs - if ($str[0] == '{') + if ($str[0] != 'a' && $str[1] != ':') { return json_decode($str, true); } // handling of old PHP serialized and addslashed prefs - $data = unserialize($str); + $data = php_safe_unserialize($str); if($data === false) { // manually retrieve the string lengths of the serialized array if unserialize failed - $data = unserialize(preg_replace_callback('!s:(\d+):"(.*?)";!s', function($matches) + $data = php_safe_unserialize(preg_replace_callback('!s:(\d+):"(.*?)";!s', function($matches) { return 's:'.mb_strlen($matches[2],'8bit').':"'.$matches[2].'";'; }, $str)); diff --git a/phpgwapi/inc/common_functions.inc.php b/phpgwapi/inc/common_functions.inc.php index d2fef07a26..9030823622 100755 --- a/phpgwapi/inc/common_functions.inc.php +++ b/phpgwapi/inc/common_functions.inc.php @@ -1529,6 +1529,61 @@ function try_lang($key,$vars=null) return class_exists(translations,false) ? translation::translate($key,$vars) : str_replace($varnames,$vars,$key); } +/** + * Unserialize a php serialized string, but only if it contains NO objects 'O:\d:"' or 'C:\d:"' pattern + * + * Should be used for all external content, to guard against exploidts. + * + * @param string $str + * @return mixed + */ +function php_safe_unserialize($str) +{ + if ((strpos($str, 'O:') !== false || strpos($str, 'C:') !== false) && + preg_match('/(^|;|{)[OC]:\d+:"/', $str)) + { + error_log(__METHOD__."('$str') contains objects --> return false"); + return false; + } + return unserialize($str); +} + +/* some test for object safe unserialisation +if (isset($_SERVER['SCRIPT_FILENAME']) && $_SERVER['SCRIPT_FILENAME'] == __FILE__) // some tests +{ + if (php_sapi_name() !== 'cli') echo "
\n";
+	foreach(array(
+		// things unsafe to unserialize
+		"O:34:\"Horde_Kolab_Server_Decorator_Clean\":2:{s:43:\"\x00Horde_Kolab_Server_Decorator_Clean\x00_server\";" => false,
+		"O:20:\"Horde_Prefs_Identity\":2:{s:9:\"\x00*\x00_prefs\";O:11:\"Horde_Prefs\":2:{s:8:\"\x00*\x00_opts\";a:1:{s:12:\"sizecallback\";" => false,
+		"a:2:{i:0;O:12:\"Horde_Config\":1:{s:13:\"\x00*\x00_oldConfig\";s:#{php_injection.length}:\"#{php_injection}\";}i:1;s:13:\"readXMLConfig\";}}" => false,
+		'a:6:{i:0;i:0;i:1;d:2;i:2;s:4:"ABCD";i:3;r:3;i:4;O:8:"my_Class":2:{s:1:"a";r:6;s:1:"b";N;};i:5;C:16:"SplObjectStorage":14:{x:i:0;m:a:0:{}}' => false,
+		// string content, safe to unserialize
+		serialize('O:8:"stdClass"') => true,
+		serialize('C:16:"SplObjectStorage"') => true,
+		serialize(array('a' => 'O:8:"stdClass"', 'b' => 'C:16:"SplObjectStorage"')) => true,
+		// false positive: failing our tests, because it has correct delimiter (^|;|{) in front of pattern :-(
+		serialize('O:8:"stdClass";C:16:"SplObjectStorage"') => true,
+	) as $str => $result)
+	{
+		if ((bool)php_safe_unserialize($str) !== $result)
+		{
+			if (!$result)
+			{
+				echo "FAILED: $str\n";
+			}
+			else
+			{
+				echo "false positive: $str\n";
+			}
+		}
+		else
+		{
+			echo "passed: $str\n";
+		}
+	}
+}*/
+
 /**
  * php5 autoload function for eGroupWare understanding the following naming schema:
  *	1. new (prefered) nameing schema: app_class_something loading app/inc/class.class_something.inc.php