Merge pull request #10 from phpbb/ticket/security-171

[ticket/security-171] Use type cast helper for json data
This commit is contained in:
Joas Schilling 2014-11-22 17:48:37 +01:00
commit 5c3b6a8559
6 changed files with 236 additions and 20 deletions

View file

@ -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'] : '',
));
}
}

View file

@ -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'];

View file

@ -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);
}

View file

@ -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;
}
}

View file

@ -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);
}
}

View file

@ -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&lt;script&gt;alert(\'foo\');&lt;/script&gt;',
'download' => 'https://www.phpbb.com/customise/db/download/104136&lt;script&gt;alert(\'foo\');&lt;/script&gt;',
'announcement' => 'https://www.phpbb.com/customise/db/extension/boardrules/&lt;script&gt;alert(\'foo\');&lt;/script&gt;',
'eol' => '&lt;script&gt;alert(\'foo\');&lt;/script&gt;',
'security' => '&lt;script&gt;alert(\'foo\');&lt;/script&gt;',
),
),
'unstable' => array (
'1.0' => array (
'current' => '1.0.1&lt;script&gt;alert(\'foo\');&lt;/script&gt;',
'download' => 'https://www.phpbb.com/customise/db/download/104136&lt;script&gt;alert(\'foo\');&lt;/script&gt;',
'announcement' => 'https://www.phpbb.com/customise/db/extension/boardrules/&lt;script&gt;alert(\'foo\');&lt;/script&gt;',
'eol' => '&lt;script&gt;alert(\'foo\');&lt;/script&gt;',
'security' => '&lt;script&gt;alert(\'foo\');&lt;/script&gt;',
),
),
)),
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&lt;script&gt;alert(\'foo\');&lt;/script&gt;',
'download' => 'https://www.phpbb.com/customise/db/download/104136&lt;script&gt;alert(\'foo\');&lt;/script&gt;',
'announcement' => 'https://www.phpbb.com/customise/db/extension/boardrules/&lt;script&gt;alert(\'foo\');&lt;/script&gt;',
'eol' => '&lt;script&gt;alert(\'foo\');&lt;/script&gt;',
'security' => '&lt;script&gt;alert(\'foo\');&lt;/script&gt;',
),
),
'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);
}
}