diff options
| -rw-r--r-- | phpBB/phpbb/extension/metadata_manager.php | 34 | ||||
| -rw-r--r-- | phpBB/phpbb/version_helper.php | 19 | ||||
| -rw-r--r-- | tests/extension/metadata_manager_test.php | 1 | ||||
| -rw-r--r-- | tests/mock/file_downloader.php | 27 | ||||
| -rw-r--r-- | tests/mock/metadata_manager.php | 2 | ||||
| -rw-r--r-- | tests/version/version_helper_remote_test.php | 173 |
6 files changed, 236 insertions, 20 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,6 +177,7 @@ 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; @@ -184,6 +185,17 @@ class metadata_manager } /** + * 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 * * @return array Contains the cleaned metadata 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/phpBB/phpbb/version_helper.php b/phpBB/phpbb/version_helper.php index e34bd0ba60..7387ad296e 100644 --- a/phpBB/phpbb/version_helper.php +++ b/phpBB/phpbb/version_helper.php @@ -270,6 +270,16 @@ class version_helper $info = json_decode($info, true); + // Sanitize any data we retrieve from a server + if (!empty($info)) + { + $json_sanitizer = function (&$value, $key) { + $type_cast_helper = new \phpbb\request\type_cast_helper(); + $type_cast_helper->set_var($value, $value, gettype($value), true); + }; + array_walk_recursive($info, $json_sanitizer); + } + if (empty($info['stable']) && empty($info['unstable'])) { $this->user->add_lang('acp/common'); @@ -277,15 +287,6 @@ class version_helper throw new \RuntimeException($this->user->lang('VERSIONCHECK_FAIL')); } - // Replace & with & on announcement links - foreach ($info as $stability => $branches) - { - foreach ($branches as $branch => $branch_data) - { - $info[$stability][$branch]['announcement'] = (!empty($branch_data['announcement'])) ? str_replace('&', '&', $branch_data['announcement']) : ''; - } - } - $info['stable'] = (empty($info['stable'])) ? array() : $info['stable']; $info['unstable'] = (empty($info['unstable'])) ? $info['stable'] : $info['unstable']; diff --git a/tests/extension/metadata_manager_test.php b/tests/extension/metadata_manager_test.php index ead0076d90..b7a6cd3c5b 100644 --- a/tests/extension/metadata_manager_test.php +++ b/tests/extension/metadata_manager_test.php @@ -137,6 +137,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/file_downloader.php b/tests/mock/file_downloader.php new file mode 100644 index 0000000000..d8951cebf6 --- /dev/null +++ b/tests/mock/file_downloader.php @@ -0,0 +1,27 @@ +<?php +/** +* +* This file is part of the phpBB Forum Software package. +* +* @copyright (c) phpBB Limited <https://www.phpbb.com> +* @license GNU General Public License, version 2 (GPL-2.0) +* +* For full copyright and license information, please see +* the docs/CREDITS.txt file. +* +*/ + +class phpbb_mock_file_downloader extends \phpbb\file_downloader +{ + public $data; + + public function set($data) + { + $this->data = $data; + } + + public function get($host, $directory, $filename, $port = 80, $timeout = 6) + { + return $this->data; + } +} 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); } } diff --git a/tests/version/version_helper_remote_test.php b/tests/version/version_helper_remote_test.php new file mode 100644 index 0000000000..65ae7646b9 --- /dev/null +++ b/tests/version/version_helper_remote_test.php @@ -0,0 +1,173 @@ +<?php +/** + * + * This file is part of the phpBB Forum Software package. + * + * @copyright (c) phpBB Limited <https://www.phpbb.com> + * @license GNU General Public License, version 2 (GPL-2.0) + * + * For full copyright and license information, please see + * the docs/CREDITS.txt file. + * + */ + +class version_helper_remote_test extends \phpbb_test_case +{ + protected $file_downloader; + protected $cache; + protected $version_helper; + + public function setUp() + { + parent::setUp(); + + global $phpbb_root_path, $phpEx; + + include_once($phpbb_root_path . 'includes/functions.' . $phpEx); + + $config = new \phpbb\config\config(array( + 'version' => '3.1.0', + )); + $container = new \phpbb_mock_container_builder(); + $db = new \phpbb\db\driver\factory($container); + $this->cache = $this->getMock('\phpbb\cache\service', array('get'), array(new \phpbb\cache\driver\null(), $config, $db, '../../', 'php')); + $this->cache->expects($this->any()) + ->method('get') + ->with($this->anything()) + ->will($this->returnValue(false)); + $this->file_downloader = new phpbb_mock_file_downloader(); + + $this->version_helper = new \phpbb\version_helper( + $this->cache, + $config, + $this->file_downloader, + new \phpbb\user('\phpbb\datetime') + ); + $this->user = new \phpbb\user('\phpbb\datetime'); + $this->user->add_lang('acp/common'); + } + + public function provider_get_versions() + { + return array( + array('', false), + array('foobar', false), + array('{ + "stable": { + "1.0": { + "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 + } + } +}', true, array ( + 'stable' => array ( + '1.0' => array ( + '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, + ), + ), + 'unstable' => array ( + '1.0' => array ( + '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, + ), + ), + )), + array('{ + "foobar": { + "1.0": { + "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 + } + } +}', false), + array('{ + "stable": { + "1.0": { + "current": "1.0.1<script>alert(\'foo\');</script>", + "download": "https://www.phpbb.com/customise/db/download/104136<script>alert(\'foo\');</script>", + "announcement": "https://www.phpbb.com/customise/db/extension/boardrules/<script>alert(\'foo\');</script>", + "eol": "<script>alert(\'foo\');</script>", + "security": "<script>alert(\'foo\');</script>" + } + } +}', true, array ( + 'stable' => array ( + '1.0' => array ( + 'current' => '1.0.1<script>alert(\'foo\');</script>', + 'download' => 'https://www.phpbb.com/customise/db/download/104136<script>alert(\'foo\');</script>', + 'announcement' => 'https://www.phpbb.com/customise/db/extension/boardrules/<script>alert(\'foo\');</script>', + 'eol' => '<script>alert(\'foo\');</script>', + 'security' => '<script>alert(\'foo\');</script>', + ), + ), + 'unstable' => array ( + '1.0' => array ( + 'current' => '1.0.1<script>alert(\'foo\');</script>', + 'download' => 'https://www.phpbb.com/customise/db/download/104136<script>alert(\'foo\');</script>', + 'announcement' => 'https://www.phpbb.com/customise/db/extension/boardrules/<script>alert(\'foo\');</script>', + 'eol' => '<script>alert(\'foo\');</script>', + 'security' => '<script>alert(\'foo\');</script>', + ), + ), + )), + array('{ + "unstable": { + "1.0": { + "current": "1.0.1<script>alert(\'foo\');</script>", + "download": "https://www.phpbb.com/customise/db/download/104136<script>alert(\'foo\');</script>", + "announcement": "https://www.phpbb.com/customise/db/extension/boardrules/<script>alert(\'foo\');</script>", + "eol": "<script>alert(\'foo\');</script>", + "security": "<script>alert(\'foo\');</script>" + } + } +}', true, array ( + 'unstable' => array ( + '1.0' => array ( + 'current' => '1.0.1<script>alert(\'foo\');</script>', + 'download' => 'https://www.phpbb.com/customise/db/download/104136<script>alert(\'foo\');</script>', + 'announcement' => 'https://www.phpbb.com/customise/db/extension/boardrules/<script>alert(\'foo\');</script>', + 'eol' => '<script>alert(\'foo\');</script>', + 'security' => '<script>alert(\'foo\');</script>', + ), + ), + 'stable' => array(), + )), + ); + } + + /** + * @dataProvider provider_get_versions + */ + public function test_get_versions($input, $valid_data, $expected_return = '') + { + $this->file_downloader->set($input); + + if (!$valid_data) + { + try { + $return = $this->version_helper->get_versions(); + } catch (\RuntimeException $e) { + $this->assertEquals((string)$e->getMessage(), $this->user->lang('VERSIONCHECK_FAIL')); + } + } + else + { + $return = $this->version_helper->get_versions(); + } + + $this->assertEquals($expected_return, $return); + } +} |