aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorMarc Alexander <admin@m-a-styles.de>2017-07-09 16:24:49 +0200
committerMarc Alexander <admin@m-a-styles.de>2017-07-09 16:24:49 +0200
commit3fc3f55d349c9022de28fed85c3bfa2d317c5075 (patch)
treebc436c3ebda0e62ebbb8fbbe8d4ef360b2d3ef75
parentaef138d8bc2cc2c74bc9951d136c7bc1e0cf3ad7 (diff)
parenta281d526dc6cf48011c1d9e04399848f7c0c08c2 (diff)
downloadforums-3fc3f55d349c9022de28fed85c3bfa2d317c5075.tar
forums-3fc3f55d349c9022de28fed85c3bfa2d317c5075.tar.gz
forums-3fc3f55d349c9022de28fed85c3bfa2d317c5075.tar.bz2
forums-3fc3f55d349c9022de28fed85c3bfa2d317c5075.tar.xz
forums-3fc3f55d349c9022de28fed85c3bfa2d317c5075.zip
Merge branch 'ticket/security/210' into ticket/security/210-rhea
Diffstat
-rw-r--r--phpBB/phpbb/avatar/driver/remote.php7
-rw-r--r--phpBB/phpbb/avatar/driver/upload.php10
-rw-r--r--tests/avatar/manager_test.php52
3 files changed, 67 insertions, 2 deletions
diff --git a/phpBB/phpbb/avatar/driver/remote.php b/phpBB/phpbb/avatar/driver/remote.php
index 3a88a432d1..f21015ed45 100644
--- a/phpBB/phpbb/avatar/driver/remote.php
+++ b/phpBB/phpbb/avatar/driver/remote.php
@@ -85,8 +85,11 @@ class remote extends \phpbb\avatar\driver\driver
}
// Check if this url looks alright
- // This isn't perfect, but it's what phpBB 3.0 did, and might as well make sure everything is compatible
- if (!preg_match('#^(http|https|ftp)://(?:(.*?\.)*?[a-z0-9\-]+?\.[a-z]{2,4}|(?:\d{1,3}\.){3,5}\d{1,3}):?([0-9]*?).*?\.('. implode('|', $this->allowed_extensions) . ')$#i', $url))
+ // Do not allow specifying the port (see RFC 3986) or IP addresses
+ if (!preg_match('#^(http|https|ftp)://(?:(.*?\.)*?[a-z0-9\-]+?\.[a-z]{2,4}|(?:\d{1,3}\.){3,5}\d{1,3}):?([0-9]*?).*?\.('. implode('|', $this->allowed_extensions) . ')$#i', $url) ||
+ preg_match('@^(http|https|ftp)://[^/:?#]+:[0-9]+[/:?#]@i', $url) ||
+ preg_match('#(?:(?:\d{1,2}|1\d\d|2[0-4]\d|25[0-5])\.){3}(?:\d{1,2}|1\d\d|2[0-4]\d|25[0-5])#i', $url) ||
+ preg_match('#(?:(?:(?:[\dA-F]{1,4}:){6}(?:[\dA-F]{1,4}:[\dA-F]{1,4}|(?:(?:\d{1,2}|1\d\d|2[0-4]\d|25[0-5])\.){3}(?:\d{1,2}|1\d\d|2[0-4]\d|25[0-5])))|(?:::(?:[\dA-F]{1,4}:){0,5}(?:[\dA-F]{1,4}(?::[\dA-F]{1,4})?|(?:(?:\d{1,2}|1\d\d|2[0-4]\d|25[0-5])\.){3}(?:\d{1,2}|1\d\d|2[0-4]\d|25[0-5])))|(?:(?:[\dA-F]{1,4}:):(?:[\dA-F]{1,4}:){4}(?:[\dA-F]{1,4}:[\dA-F]{1,4}|(?:(?:\d{1,2}|1\d\d|2[0-4]\d|25[0-5])\.){3}(?:\d{1,2}|1\d\d|2[0-4]\d|25[0-5])))|(?:(?:[\dA-F]{1,4}:){1,2}:(?:[\dA-F]{1,4}:){3}(?:[\dA-F]{1,4}:[\dA-F]{1,4}|(?:(?:\d{1,2}|1\d\d|2[0-4]\d|25[0-5])\.){3}(?:\d{1,2}|1\d\d|2[0-4]\d|25[0-5])))|(?:(?:[\dA-F]{1,4}:){1,3}:(?:[\dA-F]{1,4}:){2}(?:[\dA-F]{1,4}:[\dA-F]{1,4}|(?:(?:\d{1,2}|1\d\d|2[0-4]\d|25[0-5])\.){3}(?:\d{1,2}|1\d\d|2[0-4]\d|25[0-5])))|(?:(?:[\dA-F]{1,4}:){1,4}:(?:[\dA-F]{1,4}:)(?:[\dA-F]{1,4}:[\dA-F]{1,4}|(?:(?:\d{1,2}|1\d\d|2[0-4]\d|25[0-5])\.){3}(?:\d{1,2}|1\d\d|2[0-4]\d|25[0-5])))|(?:(?:[\dA-F]{1,4}:){1,5}:(?:[\dA-F]{1,4}:[\dA-F]{1,4}|(?:(?:\d{1,2}|1\d\d|2[0-4]\d|25[0-5])\.){3}(?:\d{1,2}|1\d\d|2[0-4]\d|25[0-5])))|(?:(?:[\dA-F]{1,4}:){1,6}:[\dA-F]{1,4})|(?:(?:[\dA-F]{1,4}:){1,7}:)|(?:::))#i', $url))
{
$error[] = 'AVATAR_URL_INVALID';
return false;
diff --git a/phpBB/phpbb/avatar/driver/upload.php b/phpBB/phpbb/avatar/driver/upload.php
index 4effa4c410..6526aa7c13 100644
--- a/phpBB/phpbb/avatar/driver/upload.php
+++ b/phpBB/phpbb/avatar/driver/upload.php
@@ -146,6 +146,16 @@ class upload extends \phpbb\avatar\driver\driver
return false;
}
+ // Do not allow specifying the port (see RFC 3986) or IP addresses
+ // remote_upload() will do its own check for allowed filetypes
+ if (preg_match('@^(http|https|ftp)://[^/:?#]+:[0-9]+[/:?#]@i', $url) ||
+ preg_match('#(?:(?:\d{1,2}|1\d\d|2[0-4]\d|25[0-5])\.){3}(?:\d{1,2}|1\d\d|2[0-4]\d|25[0-5])#i', $url) ||
+ preg_match('#(?:(?:(?:[\dA-F]{1,4}:){6}(?:[\dA-F]{1,4}:[\dA-F]{1,4}|(?:(?:\d{1,2}|1\d\d|2[0-4]\d|25[0-5])\.){3}(?:\d{1,2}|1\d\d|2[0-4]\d|25[0-5])))|(?:::(?:[\dA-F]{1,4}:){0,5}(?:[\dA-F]{1,4}(?::[\dA-F]{1,4})?|(?:(?:\d{1,2}|1\d\d|2[0-4]\d|25[0-5])\.){3}(?:\d{1,2}|1\d\d|2[0-4]\d|25[0-5])))|(?:(?:[\dA-F]{1,4}:):(?:[\dA-F]{1,4}:){4}(?:[\dA-F]{1,4}:[\dA-F]{1,4}|(?:(?:\d{1,2}|1\d\d|2[0-4]\d|25[0-5])\.){3}(?:\d{1,2}|1\d\d|2[0-4]\d|25[0-5])))|(?:(?:[\dA-F]{1,4}:){1,2}:(?:[\dA-F]{1,4}:){3}(?:[\dA-F]{1,4}:[\dA-F]{1,4}|(?:(?:\d{1,2}|1\d\d|2[0-4]\d|25[0-5])\.){3}(?:\d{1,2}|1\d\d|2[0-4]\d|25[0-5])))|(?:(?:[\dA-F]{1,4}:){1,3}:(?:[\dA-F]{1,4}:){2}(?:[\dA-F]{1,4}:[\dA-F]{1,4}|(?:(?:\d{1,2}|1\d\d|2[0-4]\d|25[0-5])\.){3}(?:\d{1,2}|1\d\d|2[0-4]\d|25[0-5])))|(?:(?:[\dA-F]{1,4}:){1,4}:(?:[\dA-F]{1,4}:)(?:[\dA-F]{1,4}:[\dA-F]{1,4}|(?:(?:\d{1,2}|1\d\d|2[0-4]\d|25[0-5])\.){3}(?:\d{1,2}|1\d\d|2[0-4]\d|25[0-5])))|(?:(?:[\dA-F]{1,4}:){1,5}:(?:[\dA-F]{1,4}:[\dA-F]{1,4}|(?:(?:\d{1,2}|1\d\d|2[0-4]\d|25[0-5])\.){3}(?:\d{1,2}|1\d\d|2[0-4]\d|25[0-5])))|(?:(?:[\dA-F]{1,4}:){1,6}:[\dA-F]{1,4})|(?:(?:[\dA-F]{1,4}:){1,7}:)|(?:::))#i', $url))
+ {
+ $error[] = 'AVATAR_URL_INVALID';
+ return false;
+ }
+
$file = $upload->handle_upload('files.types.remote', $url);
}
else
diff --git a/tests/avatar/manager_test.php b/tests/avatar/manager_test.php
index 924f1319a2..aff6160a95 100644
--- a/tests/avatar/manager_test.php
+++ b/tests/avatar/manager_test.php
@@ -384,4 +384,56 @@ class phpbb_avatar_manager_test extends \phpbb_database_test_case
'avatar_height' => 0,
), $row);
}
+
+ public function data_remote_avatar_url()
+ {
+ return array(
+ array('127.0.0.1:91?foo.jpg', 80, 80, array('AVATAR_URL_INVALID')),
+ array(gethostbyname('secure.gravatar.com') . '/avatar/55502f40dc8b7c769880b10874abc9d0.jpg', 80, 80),
+ array('secure.gravatar.com/avatar/55502f40dc8b7c769880b10874abc9d0.jpg', 80, 80),
+ array(gethostbyname('secure.gravatar.com') . ':120/avatar/55502f40dc8b7c769880b10874abc9d0.jpg', 80, 80, array('AVATAR_URL_INVALID')),
+ array('secure.gravatar.com:80/avatar/55502f40dc8b7c769880b10874abc9d0.jpg', 80, 80, array('AVATAR_URL_INVALID')),
+ array('secure.gravatar.com:80?55502f40dc8b7c769880b10874abc9d0.jpg', 80, 80, array('AVATAR_URL_INVALID')),
+ array('secure.gravatar.com?55502f40dc8b7c769880b10874abc9d0.jpg', 80, 80, array('AVATAR_URL_INVALID')), // should be a 404
+ );
+ }
+
+ /**
+ * @dataProvider data_remote_avatar_url
+ */
+ public function test_remote_avatar_url($url, $width, $height, $expected_error = array())
+ {
+ global $phpbb_root_path, $phpEx;
+
+ if (!function_exists('get_preg_expression'))
+ {
+ require($phpbb_root_path . 'includes/functions.' . $phpEx);
+ }
+
+ $this->config['server_name'] = 'foobar.com';
+
+ /** @var \phpbb\avatar\driver\remote $remote_avatar */
+ $remote_avatar = $this->manager->get_driver('avatar.driver.remote', false);
+
+ $request = new phpbb_mock_request(array(), array(
+ 'avatar_remote_url' => $url,
+ 'avatar_remote_width' => $width,
+ 'avatar_remote_height' => $height,
+ ));
+
+ $user = new \phpbb\user('\phpbb\datetime');
+ $row = array();
+ $error = array();
+
+ $return = $remote_avatar->process_form($request, null, $user, $row, $error);
+ if (count($expected_error) > 0)
+ {
+ $this->assertFalse($return);
+ }
+ else
+ {
+ $this->assertNotEquals(false, $return);
+ }
+ $this->assertSame($expected_error, $error);
+ }
}
generated by cgit v1.2.1 (git 2.21.0) at 2024-11-11 08:51:25 +0000