From ad251e4590744b0927019ae935c92c7101aa7678 Mon Sep 17 00:00:00 2001
From: Marc Alexander <admin@m-a-styles.de>
Date: Tue, 27 Dec 2016 18:11:31 +0100
Subject: [PATCH] [ticket/security-203] Do not add null values to versions info

Also stopped using reference for validate_versions() method argument.

SECURTIY-203
---
 phpBB/phpbb/version_helper.php               | 15 ++++++++-------
 tests/version/version_helper_remote_test.php | 15 ++++++++++++++-
 2 files changed, 22 insertions(+), 8 deletions(-)

diff --git a/phpBB/phpbb/version_helper.php b/phpBB/phpbb/version_helper.php
index dc95f6d001..e2d90af04a 100644
--- a/phpBB/phpbb/version_helper.php
+++ b/phpBB/phpbb/version_helper.php
@@ -315,7 +315,7 @@ class version_helper
 			$info['stable'] = (empty($info['stable'])) ? array() : $info['stable'];
 			$info['unstable'] = (empty($info['unstable'])) ? $info['stable'] : $info['unstable'];
 
-			$this->validate_versions($info);
+			$info = $this->validate_versions($info);
 
 			$this->cache->put($cache_file, $info, 86400); // 24 hours
 		}
@@ -328,8 +328,10 @@ class version_helper
 	 *
 	 * @param array $versions_info Decoded json data array. Will be modified
 	 *		and cleaned by this method
+	 *
+	 * @return array Versions info array
 	 */
-	public function validate_versions(&$versions_info)
+	public function validate_versions($versions_info)
 	{
 		$array_diff = array_diff_key($versions_info, array($this->version_schema));
 
@@ -362,7 +364,7 @@ class version_helper
 					$version_data = array();
 					foreach ($this->version_schema[$stability_type] as $key => $value)
 					{
-						if (isset($old_version_data[$key]) || $old_version_data[$key] === null)
+						if (isset($old_version_data[$key]))
 						{
 							$version_data[$key] = $old_version_data[$key];
 						}
@@ -388,16 +390,13 @@ class version_helper
 							if (!empty($value) && !preg_match('#^' . get_preg_expression('url') . '$#iu', $value) &&
 								!preg_match('#^' . get_preg_expression('www_url') . '$#iu', $value))
 							{
-								$value = '';
 								throw new \RuntimeException($this->user->lang('VERSIONCHECK_INVALID_URL'));
 							}
 						break;
 
 						case 'version':
-							$value = $value ?: '';
-							if (!preg_match(get_preg_expression('semantic_version'), $value))
+							if (!empty($value) && !preg_match(get_preg_expression('semantic_version'), $value))
 							{
-								$value = '';
 								throw new \RuntimeException($this->user->lang('VERSIONCHECK_INVALID_VERSION'));
 							}
 						break;
@@ -409,5 +408,7 @@ class version_helper
 				}
 			}
 		}
+
+		return $versions_info;
 	}
 }
diff --git a/tests/version/version_helper_remote_test.php b/tests/version/version_helper_remote_test.php
index 596b7194de..b2d497b72a 100644
--- a/tests/version/version_helper_remote_test.php
+++ b/tests/version/version_helper_remote_test.php
@@ -172,7 +172,20 @@ class version_helper_remote_test extends \phpbb_test_case
 				'current' => '1.0.1',
 				'download'	=> 'https://www.phpbb.com/customise/db/download/104136',
 				'announcement'	=> 'https://www.phpbb.com/customise/db/extension/boardrules/',
-				'eol'		=> null,
+				'security'	=> false,
+			))), 'VERSIONCHECK_INVALID_ENTRY'),
+			array('{
+    "unstable": {
+        "1.0": {
+            "current<script>alert(\'foo\');</script>": "1.0.1",
+            "download2": "https://www.phpbb.com/customise/db/download/104136",
+            "bannouncement": "https://www.phpbb.com/customise/db/extension/boardrules/",
+            "eol": null,
+            "security": false,
+            "foobar": "<script>alert(\'test\');<script>"
+        }
+    }
+}', true, array('stable' => array(), 'unstable' => array('1.0' => array(
 				'security'	=> false,
 			))), 'VERSIONCHECK_INVALID_ENTRY'),
 		);