Skip to content

Commit 5966323

Browse files
committed
[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
1 parent fa256ae commit 5966323

File tree

1 file changed

+107
-0
lines changed

1 file changed

+107
-0
lines changed

version_helper.php

Lines changed: 107 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,23 @@ class version_helper
6161
/** @var \phpbb\user */
6262
protected $user;
6363

64+
protected $version_schema = array(
65+
'stable' => array(
66+
'current' => 'version',
67+
'download' => 'url',
68+
'announcement' => 'url',
69+
'eol' => 'url',
70+
'security' => 'bool',
71+
),
72+
'unstable' => array(
73+
'current' => 'version',
74+
'download' => 'url',
75+
'announcement' => 'url',
76+
'eol' => 'url',
77+
'security' => 'bool',
78+
),
79+
);
80+
6481
/**
6582
* Constructor
6683
*
@@ -298,9 +315,99 @@ public function get_versions($force_update = false, $force_cache = false)
298315
$info['stable'] = (empty($info['stable'])) ? array() : $info['stable'];
299316
$info['unstable'] = (empty($info['unstable'])) ? $info['stable'] : $info['unstable'];
300317

318+
$this->validate_versions($info);
319+
301320
$this->cache->put($cache_file, $info, 86400); // 24 hours
302321
}
303322

304323
return $info;
305324
}
325+
326+
/**
327+
* Validate versions info input
328+
*
329+
* @param array $versions_info Decoded json data array. Will be modified
330+
* and cleaned by this method
331+
*/
332+
public function validate_versions(&$versions_info)
333+
{
334+
$array_diff = array_diff_key($versions_info, array($this->version_schema));
335+
336+
// Remove excessive data
337+
if (count($array_diff) > 0)
338+
{
339+
$old_versions_info = $versions_info;
340+
$versions_info = array(
341+
'stable' => !empty($old_versions_info['stable']) ? $old_versions_info['stable'] : array(),
342+
'unstable' => !empty($old_versions_info['unstable']) ? $old_versions_info['unstable'] : array(),
343+
);
344+
unset($old_versions_info);
345+
}
346+
347+
foreach ($versions_info as $stability_type => &$versions_data)
348+
{
349+
foreach ($versions_data as $branch => &$version_data)
350+
{
351+
if (!preg_match('/^[0-9]+\.[0-9]+$/', $branch))
352+
{
353+
unset($versions_data[$branch]);
354+
continue;
355+
}
356+
357+
$stability_diff = array_diff_key($version_data, $this->version_schema[$stability_type]);
358+
359+
if (count($stability_diff) > 0)
360+
{
361+
$old_version_data = $version_data;
362+
$version_data = array();
363+
foreach ($this->version_schema[$stability_type] as $key => $value)
364+
{
365+
if (isset($old_version_data[$key]) || $old_version_data[$key] === null)
366+
{
367+
$version_data[$key] = $old_version_data[$key];
368+
}
369+
}
370+
unset($old_version_data);
371+
}
372+
373+
foreach ($version_data as $key => &$value)
374+
{
375+
if (!isset($this->version_schema[$stability_type][$key]))
376+
{
377+
unset($version_data[$key]);
378+
throw new \RuntimeException($this->user->lang('VERSIONCHECK_INVALID_ENTRY'));
379+
}
380+
381+
switch ($this->version_schema[$stability_type][$key])
382+
{
383+
case 'bool':
384+
$value = (bool) $value;
385+
break;
386+
387+
case 'url':
388+
if (!empty($value) && !preg_match('#^' . get_preg_expression('url') . '$#iu', $value) &&
389+
!preg_match('#^' . get_preg_expression('www_url') . '$#iu', $value))
390+
{
391+
$value = '';
392+
throw new \RuntimeException($this->user->lang('VERSIONCHECK_INVALID_URL'));
393+
}
394+
break;
395+
396+
case 'version':
397+
$value = $value ?: '';
398+
if (!preg_match(get_preg_expression('semantic_version'), $value))
399+
{
400+
$value = '';
401+
throw new \RuntimeException($this->user->lang('VERSIONCHECK_INVALID_VERSION'));
402+
}
403+
break;
404+
405+
default:
406+
// Shouldn't be possible to trigger this
407+
throw new \RuntimeException($this->user->lang('VERSIONCHECK_INVALID_ENTRY'));
408+
}
409+
}
410+
}
411+
}
412+
}
306413
}

0 commit comments

Comments
 (0)