From 34004612acb0c55f1cf86271d5a62c1b396ee829 Mon Sep 17 00:00:00 2001
From: Marc Alexander <admin@m-a-styles.de>
Date: Sun, 16 Nov 2014 13:09:03 +0100
Subject: [PATCH] [ticket/security-171] Sanitize data from composer.json

SECURITY-171
---
 phpBB/phpbb/extension/metadata_manager.php | 34 +++++++++++++++-------
 tests/extension/metadata_manager_test.php  |  1 +
 tests/mock/metadata_manager.php            |  2 ++
 3 files changed, 26 insertions(+), 11 deletions(-)

diff --git a/phpBB/phpbb/extension/metadata_manager.php b/phpBB/phpbb/extension/metadata_manager.php
index edca8ee1af..a64d88fe39 100644
--- a/phpBB/phpbb/extension/metadata_manager.php
+++ b/phpBB/phpbb/extension/metadata_manager.php
@@ -177,12 +177,24 @@ class metadata_manager
 				throw new \phpbb\extension\exception($this->user->lang('FILE_JSON_DECODE_ERR', $this->metadata_file));
 			}
 
+			array_walk_recursive($metadata, array($this, 'sanitize_json'));
 			$this->metadata = $metadata;
 
 			return true;
 		}
 	}
 
+	/**
+	 * Sanitize input from JSON array using htmlspecialchars()
+	 *
+	 * @param mixed		$value	Value of array row
+	 * @param string	$key	Key of array row
+	 */
+	public function sanitize_json(&$value, $key)
+	{
+		$value = htmlspecialchars($value);
+	}
+
 	/**
 	* This array handles the cleaning of the array
 	*
@@ -337,30 +349,30 @@ class metadata_manager
 	public function output_template_data()
 	{
 		$this->template->assign_vars(array(
-			'META_NAME'			=> htmlspecialchars($this->metadata['name']),
-			'META_TYPE'			=> htmlspecialchars($this->metadata['type']),
-			'META_DESCRIPTION'	=> (isset($this->metadata['description'])) ? htmlspecialchars($this->metadata['description']) : '',
+			'META_NAME'			=> $this->metadata['name'],
+			'META_TYPE'			=> $this->metadata['type'],
+			'META_DESCRIPTION'	=> (isset($this->metadata['description'])) ? $this->metadata['description'] : '',
 			'META_HOMEPAGE'		=> (isset($this->metadata['homepage'])) ? $this->metadata['homepage'] : '',
-			'META_VERSION'		=> (isset($this->metadata['version'])) ? htmlspecialchars($this->metadata['version']) : '',
-			'META_TIME'			=> (isset($this->metadata['time'])) ? htmlspecialchars($this->metadata['time']) : '',
-			'META_LICENSE'		=> htmlspecialchars($this->metadata['license']),
+			'META_VERSION'		=> (isset($this->metadata['version'])) ? $this->metadata['version'] : '',
+			'META_TIME'			=> (isset($this->metadata['time'])) ? $this->metadata['time'] : '',
+			'META_LICENSE'		=> $this->metadata['license'],
 
-			'META_REQUIRE_PHP'		=> (isset($this->metadata['require']['php'])) ? htmlspecialchars($this->metadata['require']['php']) : '',
+			'META_REQUIRE_PHP'		=> (isset($this->metadata['require']['php'])) ? $this->metadata['require']['php'] : '',
 			'META_REQUIRE_PHP_FAIL'	=> !$this->validate_require_php(),
 
-			'META_REQUIRE_PHPBB'		=> (isset($this->metadata['extra']['soft-require']['phpbb/phpbb'])) ? htmlspecialchars($this->metadata['extra']['soft-require']['phpbb/phpbb']) : '',
+			'META_REQUIRE_PHPBB'		=> (isset($this->metadata['extra']['soft-require']['phpbb/phpbb'])) ? $this->metadata['extra']['soft-require']['phpbb/phpbb'] : '',
 			'META_REQUIRE_PHPBB_FAIL'	=> !$this->validate_require_phpbb(),
 
-			'META_DISPLAY_NAME'	=> (isset($this->metadata['extra']['display-name'])) ? htmlspecialchars($this->metadata['extra']['display-name']) : '',
+			'META_DISPLAY_NAME'	=> (isset($this->metadata['extra']['display-name'])) ? $this->metadata['extra']['display-name'] : '',
 		));
 
 		foreach ($this->metadata['authors'] as $author)
 		{
 			$this->template->assign_block_vars('meta_authors', array(
-				'AUTHOR_NAME'		=> htmlspecialchars($author['name']),
+				'AUTHOR_NAME'		=> $author['name'],
 				'AUTHOR_EMAIL'		=> (isset($author['email'])) ? $author['email'] : '',
 				'AUTHOR_HOMEPAGE'	=> (isset($author['homepage'])) ? $author['homepage'] : '',
-				'AUTHOR_ROLE'		=> (isset($author['role'])) ? htmlspecialchars($author['role']) : '',
+				'AUTHOR_ROLE'		=> (isset($author['role'])) ? $author['role'] : '',
 			));
 		}
 	}
diff --git a/tests/extension/metadata_manager_test.php b/tests/extension/metadata_manager_test.php
index 8e27b39459..fab1d3af3a 100644
--- a/tests/extension/metadata_manager_test.php
+++ b/tests/extension/metadata_manager_test.php
@@ -123,6 +123,7 @@ class phpbb_extension_metadata_manager_test extends phpbb_database_test_case
 		}
 
 		$json = json_decode(file_get_contents($this->phpbb_root_path . 'ext/vendor2/foo/composer.json'), true);
+		array_walk_recursive($json, array($manager, 'sanitize_json'));
 
 		$this->assertEquals($metadata, $json);
 	}
diff --git a/tests/mock/metadata_manager.php b/tests/mock/metadata_manager.php
index 16900a0fc1..2443fad560 100644
--- a/tests/mock/metadata_manager.php
+++ b/tests/mock/metadata_manager.php
@@ -15,11 +15,13 @@ class phpbb_mock_metadata_manager extends \phpbb\extension\metadata_manager
 {
 	public function set_metadata($metadata)
 	{
+		array_walk_recursive($metadata, array($this, 'sanitize_json'));
 		$this->metadata = $metadata;
 	}
 
 	public function merge_metadata($metadata)
 	{
+		array_walk_recursive($metadata, array($this, 'sanitize_json'));
 		$this->metadata = array_merge($this->metadata, $metadata);
 	}
 }