mirror of
https://github.com/phpbb/phpbb.git
synced 2025-03-14 04:30:29 +01:00
[ticket/security-203] Fully validate version check data in version helper
This will also take care of SECURITY-204 as it's the same underlying issue. Admins still need to ensure they don't visit malicious sites for URLs provided by extensions. SECURITY-203
This commit is contained in:
parent
97a0f49be4
commit
658820654f
@ -3442,6 +3442,11 @@ function get_preg_expression($mode)
|
||||
case 'path_remove_dot_trailing_slash':
|
||||
return '#^(?:(\.)?)+(?:(.+)?)+(?:([\\/\\\])$)#';
|
||||
break;
|
||||
|
||||
case 'semantic_version':
|
||||
// Regular expression to match semantic versions by http://rgxdb.com/
|
||||
return '/(?<=^[Vv]|^)(?:(?<major>(?:0|[1-9](?:(?:0|[1-9])+)*))[.](?<minor>(?:0|[1-9](?:(?:0|[1-9])+)*))[.](?<patch>(?:0|[1-9](?:(?:0|[1-9])+)*))(?:-(?<prerelease>(?:(?:(?:[A-Za-z]|-)(?:(?:(?:0|[1-9])|(?:[A-Za-z]|-))+)?|(?:(?:(?:0|[1-9])|(?:[A-Za-z]|-))+)(?:[A-Za-z]|-)(?:(?:(?:0|[1-9])|(?:[A-Za-z]|-))+)?)|(?:0|[1-9](?:(?:0|[1-9])+)*))(?:[.](?:(?:(?:[A-Za-z]|-)(?:(?:(?:0|[1-9])|(?:[A-Za-z]|-))+)?|(?:(?:(?:0|[1-9])|(?:[A-Za-z]|-))+)(?:[A-Za-z]|-)(?:(?:(?:0|[1-9])|(?:[A-Za-z]|-))+)?)|(?:0|[1-9](?:(?:0|[1-9])+)*)))*))?(?:[+](?<build>(?:(?:(?:[A-Za-z]|-)(?:(?:(?:0|[1-9])|(?:[A-Za-z]|-))+)?|(?:(?:(?:0|[1-9])|(?:[A-Za-z]|-))+)(?:[A-Za-z]|-)(?:(?:(?:0|[1-9])|(?:[A-Za-z]|-))+)?)|(?:(?:0|[1-9])+))(?:[.](?:(?:(?:[A-Za-z]|-)(?:(?:(?:0|[1-9])|(?:[A-Za-z]|-))+)?|(?:(?:(?:0|[1-9])|(?:[A-Za-z]|-))+)(?:[A-Za-z]|-)(?:(?:(?:0|[1-9])|(?:[A-Za-z]|-))+)?)|(?:(?:0|[1-9])+)))*))?)$/';
|
||||
break;
|
||||
}
|
||||
|
||||
return '';
|
||||
|
@ -417,11 +417,14 @@ $lang = array_merge($lang, array(
|
||||
'UPLOAD_DIR_SIZE' => 'Size of posted attachments',
|
||||
'USERS_PER_DAY' => 'Users per day',
|
||||
|
||||
'VALUE' => 'Value',
|
||||
'VERSIONCHECK_FAIL' => 'Failed to obtain latest version information.',
|
||||
'VERSIONCHECK_FORCE_UPDATE' => 'Re-Check version',
|
||||
'VIEW_ADMIN_LOG' => 'View administrator log',
|
||||
'VIEW_INACTIVE_USERS' => 'View inactive users',
|
||||
'VALUE' => 'Value',
|
||||
'VERSIONCHECK_FAIL' => 'Failed to obtain latest version information.',
|
||||
'VERSIONCHECK_FORCE_UPDATE' => 'Re-Check version',
|
||||
'VERSIONCHECK_INVALID_ENTRY' => 'Latest version information contains an unsupported entry.',
|
||||
'VERSIONCHECK_INVALID_URL' => 'Latest version information contains invalid URL.',
|
||||
'VERSIONCHECK_INVALID_VERSION' => 'Latest version information contains an invalid version.',
|
||||
'VIEW_ADMIN_LOG' => 'View administrator log',
|
||||
'VIEW_INACTIVE_USERS' => 'View inactive users',
|
||||
|
||||
'WELCOME_PHPBB' => 'Welcome to phpBB',
|
||||
'WRITABLE_CONFIG' => 'Your config file (config.php) is currently world-writable. We strongly encourage you to change the permissions to 640 or at least to 644 (for example: <a href="http://en.wikipedia.org/wiki/Chmod" rel="external">chmod</a> 640 config.php).',
|
||||
|
@ -61,6 +61,23 @@ class version_helper
|
||||
/** @var \phpbb\user */
|
||||
protected $user;
|
||||
|
||||
protected $version_schema = array(
|
||||
'stable' => array(
|
||||
'current' => 'version',
|
||||
'download' => 'url',
|
||||
'announcement' => 'url',
|
||||
'eol' => 'url',
|
||||
'security' => 'bool',
|
||||
),
|
||||
'unstable' => array(
|
||||
'current' => 'version',
|
||||
'download' => 'url',
|
||||
'announcement' => 'url',
|
||||
'eol' => 'url',
|
||||
'security' => 'bool',
|
||||
),
|
||||
);
|
||||
|
||||
/**
|
||||
* Constructor
|
||||
*
|
||||
@ -298,9 +315,99 @@ class version_helper
|
||||
$info['stable'] = (empty($info['stable'])) ? array() : $info['stable'];
|
||||
$info['unstable'] = (empty($info['unstable'])) ? $info['stable'] : $info['unstable'];
|
||||
|
||||
$this->validate_versions($info);
|
||||
|
||||
$this->cache->put($cache_file, $info, 86400); // 24 hours
|
||||
}
|
||||
|
||||
return $info;
|
||||
}
|
||||
|
||||
/**
|
||||
* Validate versions info input
|
||||
*
|
||||
* @param array $versions_info Decoded json data array. Will be modified
|
||||
* and cleaned by this method
|
||||
*/
|
||||
public function validate_versions(&$versions_info)
|
||||
{
|
||||
$array_diff = array_diff_key($versions_info, array($this->version_schema));
|
||||
|
||||
// Remove excessive data
|
||||
if (count($array_diff) > 0)
|
||||
{
|
||||
$old_versions_info = $versions_info;
|
||||
$versions_info = array(
|
||||
'stable' => !empty($old_versions_info['stable']) ? $old_versions_info['stable'] : array(),
|
||||
'unstable' => !empty($old_versions_info['unstable']) ? $old_versions_info['unstable'] : array(),
|
||||
);
|
||||
unset($old_versions_info);
|
||||
}
|
||||
|
||||
foreach ($versions_info as $stability_type => &$versions_data)
|
||||
{
|
||||
foreach ($versions_data as $branch => &$version_data)
|
||||
{
|
||||
if (!preg_match('/^[0-9]+\.[0-9]+$/', $branch))
|
||||
{
|
||||
unset($versions_data[$branch]);
|
||||
continue;
|
||||
}
|
||||
|
||||
$stability_diff = array_diff_key($version_data, $this->version_schema[$stability_type]);
|
||||
|
||||
if (count($stability_diff) > 0)
|
||||
{
|
||||
$old_version_data = $version_data;
|
||||
$version_data = array();
|
||||
foreach ($this->version_schema[$stability_type] as $key => $value)
|
||||
{
|
||||
if (isset($old_version_data[$key]) || $old_version_data[$key] === null)
|
||||
{
|
||||
$version_data[$key] = $old_version_data[$key];
|
||||
}
|
||||
}
|
||||
unset($old_version_data);
|
||||
}
|
||||
|
||||
foreach ($version_data as $key => &$value)
|
||||
{
|
||||
if (!isset($this->version_schema[$stability_type][$key]))
|
||||
{
|
||||
unset($version_data[$key]);
|
||||
throw new \RuntimeException($this->user->lang('VERSIONCHECK_INVALID_ENTRY'));
|
||||
}
|
||||
|
||||
switch ($this->version_schema[$stability_type][$key])
|
||||
{
|
||||
case 'bool':
|
||||
$value = (bool) $value;
|
||||
break;
|
||||
|
||||
case 'url':
|
||||
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))
|
||||
{
|
||||
$value = '';
|
||||
throw new \RuntimeException($this->user->lang('VERSIONCHECK_INVALID_VERSION'));
|
||||
}
|
||||
break;
|
||||
|
||||
default:
|
||||
// Shouldn't be possible to trigger this
|
||||
throw new \RuntimeException($this->user->lang('VERSIONCHECK_INVALID_ENTRY'));
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
@ -37,21 +37,21 @@ class version_helper_remote_test extends \phpbb_test_case
|
||||
->will($this->returnValue(false));
|
||||
$this->file_downloader = new phpbb_mock_file_downloader();
|
||||
|
||||
$this->user = new \phpbb\user('\phpbb\datetime');
|
||||
$this->user->add_lang('acp/common');
|
||||
$this->version_helper = new \phpbb\version_helper(
|
||||
$this->cache,
|
||||
$config,
|
||||
$this->file_downloader,
|
||||
new \phpbb\user('\phpbb\datetime')
|
||||
$this->user
|
||||
);
|
||||
$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('', false, '', 'VERSIONCHECK_FAIL'),
|
||||
array('foobar', false, '', 'VERSIONCHECK_FAIL'),
|
||||
array('{
|
||||
"stable": {
|
||||
"1.0": {
|
||||
@ -92,7 +92,7 @@ class version_helper_remote_test extends \phpbb_test_case
|
||||
"security": false
|
||||
}
|
||||
}
|
||||
}', false),
|
||||
}', false, '', 'VERSIONCHECK_FAIL'),
|
||||
array('{
|
||||
"stable": {
|
||||
"1.0": {
|
||||
@ -103,26 +103,7 @@ class version_helper_remote_test extends \phpbb_test_case
|
||||
"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>',
|
||||
),
|
||||
),
|
||||
)),
|
||||
}', false, null, 'VERSIONCHECK_INVALID_VERSION'),
|
||||
array('{
|
||||
"unstable": {
|
||||
"1.0": {
|
||||
@ -133,25 +114,74 @@ class version_helper_remote_test extends \phpbb_test_case
|
||||
"security": "<script>alert(\'foo\');</script>"
|
||||
}
|
||||
}
|
||||
}', false, null, 'VERSIONCHECK_INVALID_VERSION'),
|
||||
array('{
|
||||
"unstable": {
|
||||
"1.0<script>alert(\'foo\');</script>": {
|
||||
"current": "1.0.1",
|
||||
"download": "https://www.phpbb.com/customise/db/download/104136",
|
||||
"announcement": "https://www.phpbb.com/customise/db/extension/boardrules/",
|
||||
"eol": "",
|
||||
"security": ""
|
||||
}
|
||||
}
|
||||
}', false, array('stable' => array(), 'unstable' => array()), 'VERSIONCHECK_INVALID_VERSION'),
|
||||
array('{
|
||||
"\"\n<script>alert(\'foo\');</script>\n": "test",
|
||||
"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 (
|
||||
'unstable' => 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>',
|
||||
'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,
|
||||
),
|
||||
),
|
||||
'stable' => array(),
|
||||
)),
|
||||
array('{
|
||||
"unstable": {
|
||||
"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,
|
||||
"foobar": "<script>alert(\'test\');<script>"
|
||||
}
|
||||
}
|
||||
}', true, array('stable' => array(), '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,
|
||||
))), 'VERSIONCHECK_INVALID_ENTRY'),
|
||||
);
|
||||
}
|
||||
|
||||
/**
|
||||
* @dataProvider provider_get_versions
|
||||
*/
|
||||
public function test_get_versions($input, $valid_data, $expected_return = '')
|
||||
public function test_get_versions($input, $valid_data, $expected_return = '', $expected_exception = '')
|
||||
{
|
||||
$this->file_downloader->set($input);
|
||||
|
||||
@ -160,7 +190,7 @@ class version_helper_remote_test extends \phpbb_test_case
|
||||
try {
|
||||
$return = $this->version_helper->get_versions();
|
||||
} catch (\RuntimeException $e) {
|
||||
$this->assertEquals((string)$e->getMessage(), $this->user->lang('VERSIONCHECK_FAIL'));
|
||||
$this->assertEquals((string)$e->getMessage(), $this->user->lang($expected_exception));
|
||||
}
|
||||
}
|
||||
else
|
||||
|
Loading…
x
Reference in New Issue
Block a user