From 19421fcdef62e50ea335967cc7e4487e7548db87 Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Mon, 2 Feb 2015 15:02:41 +0100 Subject: [ticket/13568] Validate imagick path as readable absolute path PHPBB3-13568 --- phpBB/adm/index.php | 36 ++++++++++++++++++++++++++++++++++ phpBB/includes/acp/acp_attachments.php | 2 +- 2 files changed, 37 insertions(+), 1 deletion(-) (limited to 'phpBB') diff --git a/phpBB/adm/index.php b/phpBB/adm/index.php index 85908476a1..885c8f0a1c 100644 --- a/phpBB/adm/index.php +++ b/phpBB/adm/index.php @@ -562,6 +562,42 @@ function validate_config_vars($config_vars, &$cfg_array, &$error) } break; + + // Absolute file path + case 'wapath': + case 'apath': + if (!$cfg_array[$config_name]) + { + break; + } + + $cfg_array[$config_name] = trim($cfg_array[$config_name]); + + // Make sure no NUL byte is present... + if (strpos($cfg_array[$config_name], "\0") !== false || strpos($cfg_array[$config_name], '%00') !== false) + { + $cfg_array[$config_name] = ''; + break; + } + + if (!file_exists($cfg_array[$config_name])) + { + $error[] = sprintf($user->lang['DIRECTORY_DOES_NOT_EXIST'], $cfg_array[$config_name]); + } + else if (!is_dir($cfg_array[$config_name])) + { + $error[] = sprintf($user->lang['DIRECTORY_NOT_DIR'], $cfg_array[$config_name]); + } + + // Check if the path is writable + if ($config_definition['validate'] === 'wapath') + { + if (file_exists($cfg_array[$config_name]) && !phpbb_is_writable($cfg_array[$config_name])) + { + $error[] = sprintf($user->lang['DIRECTORY_NOT_WRITABLE'], $cfg_array[$config_name]); + } + } + break; } } diff --git a/phpBB/includes/acp/acp_attachments.php b/phpBB/includes/acp/acp_attachments.php index 147783feae..325c6b63cb 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' => '  [ ' . $user->lang['SEARCH_IMAGICK'] . ' ]'), + 'img_imagick' => array('lang' => 'IMAGICK_PATH', 'validate' => 'apath', 'type' => 'text:20:200', 'explain' => true, 'append' => '  [ ' . $user->lang['SEARCH_IMAGICK'] . ' ]'), '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']), ) -- cgit v1.2.1 From a93df0e5112962503abc88469c1cc77cef862745 Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Mon, 2 Feb 2015 18:30:05 +0100 Subject: [ticket/13568] Use more descriptive validation names and merge with path block PHPBB3-13568 --- phpBB/adm/index.php | 47 ++++++---------------------------- phpBB/includes/acp/acp_attachments.php | 2 +- 2 files changed, 9 insertions(+), 40 deletions(-) (limited to 'phpBB') diff --git a/phpBB/adm/index.php b/phpBB/adm/index.php index 885c8f0a1c..52fe315cc0 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,12 +545,14 @@ function validate_config_vars($config_vars, &$cfg_array, &$error) break; } - if (!file_exists($phpbb_root_path . $cfg_array[$config_name])) + $path = ($config_definition['validate'] === 'wpath' || $config_definition['validate'] === 'path') ? $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]); } @@ -555,49 +560,13 @@ function validate_config_vars($config_vars, &$cfg_array, &$error) // Check if the path is writable if ($config_definition['validate'] == 'wpath' || $config_definition['validate'] == 'rwpath') { - 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]); } } break; - - // Absolute file path - case 'wapath': - case 'apath': - if (!$cfg_array[$config_name]) - { - break; - } - - $cfg_array[$config_name] = trim($cfg_array[$config_name]); - - // Make sure no NUL byte is present... - if (strpos($cfg_array[$config_name], "\0") !== false || strpos($cfg_array[$config_name], '%00') !== false) - { - $cfg_array[$config_name] = ''; - break; - } - - if (!file_exists($cfg_array[$config_name])) - { - $error[] = sprintf($user->lang['DIRECTORY_DOES_NOT_EXIST'], $cfg_array[$config_name]); - } - else if (!is_dir($cfg_array[$config_name])) - { - $error[] = sprintf($user->lang['DIRECTORY_NOT_DIR'], $cfg_array[$config_name]); - } - - // Check if the path is writable - if ($config_definition['validate'] === 'wapath') - { - if (file_exists($cfg_array[$config_name]) && !phpbb_is_writable($cfg_array[$config_name])) - { - $error[] = sprintf($user->lang['DIRECTORY_NOT_WRITABLE'], $cfg_array[$config_name]); - } - } - break; } } diff --git a/phpBB/includes/acp/acp_attachments.php b/phpBB/includes/acp/acp_attachments.php index 325c6b63cb..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' => 'apath', 'type' => 'text:20:200', 'explain' => true, 'append' => '  [ ' . $user->lang['SEARCH_IMAGICK'] . ' ]'), + 'img_imagick' => array('lang' => 'IMAGICK_PATH', 'validate' => 'absolute_path', 'type' => 'text:20:200', 'explain' => true, 'append' => '  [ ' . $user->lang['SEARCH_IMAGICK'] . ' ]'), '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']), ) -- cgit v1.2.1 From d50cec998ccf3782a3e1d6dd5031c76e32e60790 Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Mon, 2 Feb 2015 18:44:48 +0100 Subject: [ticket/13568] Correctly check rpath and rwpath validation options PHPBB3-13568 --- phpBB/adm/index.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'phpBB') diff --git a/phpBB/adm/index.php b/phpBB/adm/index.php index 52fe315cc0..9858d61e7f 100644 --- a/phpBB/adm/index.php +++ b/phpBB/adm/index.php @@ -545,7 +545,7 @@ function validate_config_vars($config_vars, &$cfg_array, &$error) break; } - $path = ($config_definition['validate'] === 'wpath' || $config_definition['validate'] === 'path') ? $phpbb_root_path . $cfg_array[$config_name] : $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)) { -- cgit v1.2.1 From 7c5d872344a59f0fe2323d34885e543a09111ca7 Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Fri, 6 Feb 2015 10:15:36 +0100 Subject: [ticket/13568] Also check if absolute_path_writable is writable PHPBB3-13568 --- phpBB/adm/index.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'phpBB') diff --git a/phpBB/adm/index.php b/phpBB/adm/index.php index 9858d61e7f..49c4be09dc 100644 --- a/phpBB/adm/index.php +++ b/phpBB/adm/index.php @@ -558,7 +558,7 @@ function validate_config_vars($config_vars, &$cfg_array, &$error) } // 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($path) && !phpbb_is_writable($path)) { -- cgit v1.2.1