diff options
author | Nicofuma <github@nicofuma.fr> | 2015-03-29 19:41:13 +0200 |
---|---|---|
committer | Nicofuma <github@nicofuma.fr> | 2015-03-29 19:41:13 +0200 |
commit | b9db47e3f52a8723b06e01f15c6e44052c014c42 (patch) | |
tree | a95b627f257c1682c211acdb861a6aa27a75a809 | |
parent | 1efbb5b3bbd8c0b38c0f265453514dfd87ef04f0 (diff) | |
parent | 7c5d872344a59f0fe2323d34885e543a09111ca7 (diff) | |
download | forums-b9db47e3f52a8723b06e01f15c6e44052c014c42.tar forums-b9db47e3f52a8723b06e01f15c6e44052c014c42.tar.gz forums-b9db47e3f52a8723b06e01f15c6e44052c014c42.tar.bz2 forums-b9db47e3f52a8723b06e01f15c6e44052c014c42.tar.xz forums-b9db47e3f52a8723b06e01f15c6e44052c014c42.zip |
Merge pull request #3359 from marc1706/ticket/13568
[ticket/13568] Validate imagick path as readable absolute path
closes #3359
-rw-r--r-- | phpBB/adm/index.php | 13 | ||||
-rw-r--r-- | phpBB/includes/acp/acp_attachments.php | 2 | ||||
-rw-r--r-- | tests/functional/acp_attachments_test.php | 78 |
3 files changed, 88 insertions, 5 deletions
diff --git a/phpBB/adm/index.php b/phpBB/adm/index.php index 85908476a1..49c4be09dc 100644 --- a/phpBB/adm/index.php +++ b/phpBB/adm/index.php @@ -524,6 +524,9 @@ function validate_config_vars($config_vars, &$cfg_array, &$error) $cfg_array[$config_name] = trim($destination); + // Absolute file path + case 'absolute_path': + case 'absolute_path_writable': // Path being relative (still prefixed by phpbb_root_path), but with the ability to escape the root dir... case 'path': case 'wpath': @@ -542,20 +545,22 @@ function validate_config_vars($config_vars, &$cfg_array, &$error) break; } - if (!file_exists($phpbb_root_path . $cfg_array[$config_name])) + $path = in_array($config_definition['validate'], array('wpath', 'path', 'rpath', 'rwpath')) ? $phpbb_root_path . $cfg_array[$config_name] : $cfg_array[$config_name]; + + if (!file_exists($path)) { $error[] = sprintf($user->lang['DIRECTORY_DOES_NOT_EXIST'], $cfg_array[$config_name]); } - if (file_exists($phpbb_root_path . $cfg_array[$config_name]) && !is_dir($phpbb_root_path . $cfg_array[$config_name])) + if (file_exists($path) && !is_dir($path)) { $error[] = sprintf($user->lang['DIRECTORY_NOT_DIR'], $cfg_array[$config_name]); } // Check if the path is writable - if ($config_definition['validate'] == 'wpath' || $config_definition['validate'] == 'rwpath') + if ($config_definition['validate'] == 'wpath' || $config_definition['validate'] == 'rwpath' || $config_definition['validate'] === 'absolute_path_writable') { - if (file_exists($phpbb_root_path . $cfg_array[$config_name]) && !phpbb_is_writable($phpbb_root_path . $cfg_array[$config_name])) + if (file_exists($path) && !phpbb_is_writable($path)) { $error[] = sprintf($user->lang['DIRECTORY_NOT_WRITABLE'], $cfg_array[$config_name]); } diff --git a/phpBB/includes/acp/acp_attachments.php b/phpBB/includes/acp/acp_attachments.php index 147783feae..bffe6f7db3 100644 --- a/phpBB/includes/acp/acp_attachments.php +++ b/phpBB/includes/acp/acp_attachments.php @@ -127,7 +127,7 @@ class acp_attachments 'img_create_thumbnail' => array('lang' => 'CREATE_THUMBNAIL', 'validate' => 'bool', 'type' => 'radio:yes_no', 'explain' => true), 'img_max_thumb_width' => array('lang' => 'MAX_THUMB_WIDTH', 'validate' => 'int', 'type' => 'text:7:15', 'explain' => true, 'append' => ' ' . $user->lang['PIXEL']), 'img_min_thumb_filesize' => array('lang' => 'MIN_THUMB_FILESIZE', 'validate' => 'int', 'type' => 'text:7:15', 'explain' => true, 'append' => ' ' . $user->lang['BYTES']), - 'img_imagick' => array('lang' => 'IMAGICK_PATH', 'validate' => 'path', 'type' => 'text:20:200', 'explain' => true, 'append' => ' <span>[ <a href="' . $this->u_action . '&action=imgmagick">' . $user->lang['SEARCH_IMAGICK'] . '</a> ]</span>'), + 'img_imagick' => array('lang' => 'IMAGICK_PATH', 'validate' => 'absolute_path', 'type' => 'text:20:200', 'explain' => true, 'append' => ' <span>[ <a href="' . $this->u_action . '&action=imgmagick">' . $user->lang['SEARCH_IMAGICK'] . '</a> ]</span>'), 'img_max' => array('lang' => 'MAX_IMAGE_SIZE', 'validate' => 'int', 'type' => 'dimension:3:4', 'explain' => true, 'append' => ' ' . $user->lang['PIXEL']), 'img_link' => array('lang' => 'IMAGE_LINK_SIZE', 'validate' => 'int', 'type' => 'dimension:3:4', 'explain' => true, 'append' => ' ' . $user->lang['PIXEL']), ) diff --git a/tests/functional/acp_attachments_test.php b/tests/functional/acp_attachments_test.php new file mode 100644 index 0000000000..8e810a508a --- /dev/null +++ b/tests/functional/acp_attachments_test.php @@ -0,0 +1,78 @@ +<?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. + * + */ + +/** + * @group functional + */ +class phpbb_functional_acp_attachments_test extends phpbb_functional_test_case +{ + public function data_imagick_path_linux() + { + return array( + array('/usr/bin', 'Configuration updated successfully'), + array('/usr/foobar', 'The entered path “/usr/foobar” does not exist.'), + array('/usr/bin/which', 'The entered path “/usr/bin/which” is not a directory.'), + ); + } + + /** + * @dataProvider data_imagick_path_linux + */ + public function test_imagick_path_linux($imagick_path, $expected) + { + if (strtolower(substr(PHP_OS, 0, 5)) !== 'linux') + { + $this->markTestSkipped('Unable to test linux specific paths on other OS.'); + } + + $this->login(); + $this->admin_login(); + + $crawler = self::request('GET', 'adm/index.php?i=attachments&mode=attach&sid=' . $this->sid); + + $form = $crawler->selectButton('Submit')->form(array('config[img_imagick]' => $imagick_path)); + + $crawler = self::submit($form); + $this->assertContains($expected, $crawler->filter('#main')->text()); + } + + public function data_imagick_path_windows() + { + return array( + array('C:\Windows', 'Configuration updated successfully'), + array('C:\Windows\foobar1', 'The entered path “C:\Windows\foobar1” does not exist.'), + array('C:\Windows\explorer.exe', 'The entered path “C:\Windows\explorer.exe” is not a directory.'), + ); + } + + /** + * @dataProvider data_imagick_path_windows + */ + public function test_imagick_path_windows($imagick_path, $expected) + { + if (strtolower(substr(PHP_OS, 0, 3)) !== 'win') + { + $this->markTestSkipped('Unable to test windows specific paths on other OS.'); + } + + $this->login(); + $this->admin_login(); + + $crawler = self::request('GET', 'adm/index.php?i=attachments&mode=attach&sid=' . $this->sid); + + $form = $crawler->selectButton('Submit')->form(array('config[img_imagick]' => $imagick_path)); + + $crawler = self::submit($form); + $this->assertContains($expected, $crawler->filter('#main')->text()); + } +} |