From c7f79344dbd4a23b62f7b5ac440648614a45f264 Mon Sep 17 00:00:00 2001 From: Beom Jinhyeok Date: Sat, 13 Oct 2018 09:55:43 +0900 Subject: [PATCH] =?UTF-8?q?=ED=8C=8C=EC=9D=BC=20=EC=97=85=EB=A1=9C?= =?UTF-8?q?=EB=93=9C=20=EC=8B=9C=20=EC=9C=84=EC=9E=A5=ED=95=9C=20=ED=8C=8C?= =?UTF-8?q?=EC=9D=BC=20=ED=98=95=EC=8B=9D=EC=9D=84=20=EC=9D=B4=EC=9A=A9?= =?UTF-8?q?=ED=95=9C=20=EB=B3=B4=EC=95=88=20=EB=AC=B8=EC=A0=9C=20=EC=88=98?= =?UTF-8?q?=EC=A0=95?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit https://github.com/xpressengine/xe-core/issues/2305 --- classes/context/Context.class.php | 33 +++-- classes/security/UploadFileFilter.class.php | 156 ++++++++++++++++---- classes/template/TemplateHandler.class.php | 91 ++++++++---- modules/admin/admin.admin.controller.php | 2 +- modules/file/file.controller.php | 2 +- modules/member/member.admin.controller.php | 6 +- modules/member/member.controller.php | 12 +- 7 files changed, 218 insertions(+), 84 deletions(-) diff --git a/classes/context/Context.class.php b/classes/context/Context.class.php index 73c4a261..cd6d4ea8 100644 --- a/classes/context/Context.class.php +++ b/classes/context/Context.class.php @@ -1156,15 +1156,12 @@ function _filterRequestVar($key, $val, $do_stripslashes = true, $remove_hack = f if($key === 'page' || $key === 'cpage' || substr_compare($key, 'srl', -3) === 0){ $result[$k] = !preg_match('/^[0-9,]+$/', $v) ? (int) $v : $v; } - elseif($key === 'mid' || $key === 'search_keyword'){ + elseif(in_array($key, array('mid','search_keyword','search_target','xe_validator_id'))){ $result[$k] = escape($v, false); } elseif($key === 'vid'){ $result[$k] = urlencode($v); } - elseif($key === 'xe_validator_id'){ - $result[$k] = escape($v, false); - } elseif(stripos($key, 'XE_VALIDATOR', 0) === 0){ unset($result[$k]); } @@ -1218,7 +1215,7 @@ function _setUploadedArgument(){ foreach($_FILES as $key => $val){ $tmp_name = $val['tmp_name']; if(!is_array($tmp_name)){ - if(!$tmp_name || !is_uploaded_file($tmp_name)){ + if(!UploadFileFilter::check($tmp_name, $val['name'])){ continue; } $val['name'] = htmlspecialchars($val['name'], ENT_COMPAT | ENT_HTML401, 'UTF-8', FALSE); @@ -1227,19 +1224,23 @@ function _setUploadedArgument(){ } else{ $files = array(); - $count_files = count($tmp_name); - for($i = 0; $i < $count_files; $i++){ - if($val['size'][$i] > 0){ - $file = array(); - $file['name'] = $val['name'][$i]; - $file['type'] = $val['type'][$i]; - $file['tmp_name'] = $val['tmp_name'][$i]; - $file['error'] = $val['error'][$i]; - $file['size'] = $val['size'][$i]; - $files[] = $file; + foreach ($tmp_name as $i => $j){ + if(!UploadFileFilter::check($val['tmp_name'][$i], $val['name'][$i])){ + $files = array(); + unset($_FILES[$key]); + break; } + $file = array(); + $file['name'] = $val['name'][$i]; + $file['type'] = $val['type'][$i]; + $file['tmp_name'] = $val['tmp_name'][$i]; + $file['error'] = $val['error'][$i]; + $file['size'] = $val['size'][$i]; + $files[] = $file; + } + if(count($files)){ + self::set($key, $files, true); } - if($files) $this->set($key, $files, TRUE); } } } diff --git a/classes/security/UploadFileFilter.class.php b/classes/security/UploadFileFilter.class.php index 731b7f1a..32d8000c 100644 --- a/classes/security/UploadFileFilter.class.php +++ b/classes/security/UploadFileFilter.class.php @@ -1,40 +1,136 @@ */ +/** + * @copyright Rhymix Developers and Contributors + * @link https://github.com/rhymix/rhymix + */ +class UploadFileFilter { + /** + * Generic checker + * + * @param string $file + * @param string $filename + * @return bool + */ + public static function check($file, $filename = null){ + // Return error if the file is not uploaded. + if(!$file || !file_exists($file) || !is_uploaded_file($file)){ + return false; + } -/* Copyright (C) DAOL Project */ + // Return error if the file size is zero. + if(($filesize = filesize($file)) == 0){ + return false; + } -class UploadFileFilter { - private static $_block_list = array('exec', 'system', 'passthru', 'show_source', 'phpinfo', 'fopen', 'file_get_contents', 'file_put_contents', 'fwrite', 'proc_open', 'popen'); - - public function check($file) { - // TODO: 기능개선후 enable - - return TRUE; // disable - if(!$file || !FileHandler::exists($file)) return TRUE; - return self::_check($file); + // Get the extension. + $ext = $filename ? strtolower(substr(strrchr($filename, '.'), 1)) : ''; + + // Check the first 4KB of the file for possible XML content. + $fp = fopen($file, 'rb'); + $first4kb = fread($fp, 4096); + $is_xml = preg_match('/<(?:\?xml|!DOCTYPE|html|head|body|meta|script|svg)\b/i', $first4kb); + + // Check SVG files. + if(($ext === 'svg' || $is_xml) && !self::_checkSVG($fp, 0, $filesize)){ + fclose($fp); + return false; + } + + // Check XML files. + if(($ext === 'xml' || $is_xml) && !self::_checkXML($fp, 0, $filesize)){ + fclose($fp); + return false; + } + + // Check HTML files. + if(($ext === 'html' || $ext === 'shtml' || $ext === 'xhtml' || $ext === 'phtml' || $is_xml) && !self::_checkHTML($fp, 0, $filesize)){ + fclose($fp); + return false; + } + + // Return true if everything is OK. + fclose($fp); + return true; } - - private function _check($file) { - if(!($fp = fopen($file, 'r'))) return FALSE; - - $has_php_tag = FALSE; - - while(!feof($fp)) { - $content = fread($fp, 8192); - if(FALSE === $has_php_tag) $has_php_tag = strpos($content, ' 0){ + if(preg_match($regexp, $content)) + { + return true; } + fseek($fp, min($to, $position += $block_size)); } - - fclose($fp); - - return TRUE; + return false; } } - /* End of file : UploadFileFilter.class.php */ /* Location: ./classes/security/UploadFileFilter.class.php */ diff --git a/classes/template/TemplateHandler.class.php b/classes/template/TemplateHandler.class.php index 81e31f91..72a24093 100644 --- a/classes/template/TemplateHandler.class.php +++ b/classes/template/TemplateHandler.class.php @@ -11,30 +11,25 @@ **/ class TemplateHandler { - var $compiled_path = './files/cache/template_compiled/'; ///< path of compiled caches files - - var $path = null; ///< target directory - var $filename = null; ///< target filename - var $file = null; ///< target file (fullpath) - var $xe_path = null; ///< XpressEngine base path - var $web_path = null; ///< tpl file web path - var $compiled_file = null; ///< tpl file web path - var $skipTags = null; - - var $handler_mtime = 0; + private $compiled_path = './files/cache/template_compiled/'; ///< path of compiled caches files + private $path = NULL; ///< target directory + private $filename = NULL; ///< target filename + private $file = NULL; ///< target file (fullpath) + private $xe_path = NULL; ///< XpressEngine base path + private $web_path = NULL; ///< tpl file web path + private $compiled_file = NULL; ///< tpl file web path + private $config = NULL; + private $skipTags = NULL; + private $handler_mtime = 0; /** * constructor * @return void **/ - function TemplateHandler() { - // TODO: replace this with static variable in PHP5 - global $__templatehandler_root_tpl; - - $__templatehandler_root_tpl = null; - - ini_set('pcre.jit', "0"); - $this->xe_path = rtrim(getScriptPath(), '/'); + public function __construct(){ + $this->xe_path = rtrim(preg_replace('/([^\.^\/]+)\.php$/i', '', $_SERVER['SCRIPT_NAME']), '/'); + $this->compiled_path = _DAOL_PATH_ . $this->compiled_path; + $this->config = new stdClass(); } /** @@ -185,6 +180,13 @@ function parse($buff = null) { $this->skipTags = array('marquee'); } + // reset config for this buffer (this step is necessary because we use a singleton for every template) + $previous_config = clone $this->config; + $this->config = new stdClass(); + + // detect existence of autoescape config + $this->config->autoescape = (strpos($buff, ' autoescape="') === FALSE) ? NULL : 'off'; + // replace comments $buff = preg_replace('@@s', '', $buff); @@ -195,7 +197,7 @@ function parse($buff = null) { $buff = $this->_parseInline($buff); // include, unload/load, import - $buff = preg_replace_callback('/{(@[\s\S]+?|(?=\$\w+|_{1,2}[A-Z]+|[!\(+-]|\w+(?:\(|::)|\d+|[\'"].*?[\'"]).+?)}|<(!--[#%])?(include|import|(un)?load(?(4)|(?:_js_plugin)?))(?(2)\(["\']([^"\']+)["\'])(.*?)(?(2)\)--|\/)>|(\s*)/', array($this, '_parseResource'), $buff); + $buff = preg_replace_callback('/{(@[\s\S]+?|(?=\$\w+|_{1,2}[A-Z]+|[!\(+-]|\w+(?:\(|::)|\d+|[\'"].*?[\'"]).+?)}|<(!--[#%])?(include|import|(un)?load(?(4)|(?:_js_plugin)?)|config)(?(2)\(["\']([^"\']+)["\'])(.*?)(?(2)\)--|\/)>|(\s*)/', array($this, '_parseResource'), $buff); // remove block which is a virtual tag $buff = preg_replace('@@is', '', $buff); @@ -212,6 +214,9 @@ function parse($buff = null) { // remove php script reopening $buff = preg_replace(array('/(\n|\r\n)+/', '/(;)?( )*\?\>\<\?php([\n\t ]+)?/'), array("\n", ";\n"), $buff); + // restore config to previous value + $this->config = $previous_config; + return $buff; } @@ -441,15 +446,36 @@ function _parseInline($buff) { **/ function _parseResource($m) { // {@ ... } or {$var} or {func(...)} - if($m[1]) { - if(preg_match('@^(\w+)\(@', $m[1], $mm) && !function_exists($mm[1])) return $m[0]; + if($m[1]){ + if(preg_match('@^(\w+)\(@', $m[1], $mm) && !function_exists($mm[1])){ + return $m[0]; + } - $echo = 'echo '; - if($m[1]{0} == '@') { - $echo = ''; - $m[1] = substr($m[1], 1); + if($m[1]{0} == '@'){ + $m[1] = $this->_replaceVar(substr($m[1], 1)); + return ""; + } + else{ + $escape_option = $this->config->autoescape !== null ? 'auto' : 'noescape'; + if(preg_match('@^(.+)\\|((?:auto|no)?escape)$@', $m[1], $mm)){ + $m[1] = $mm[1]; + $escape_option = $mm[2]; + } + elseif($m[1] === '$content' && preg_match('@/layouts/.+/layout\.html$@', $this->file)){ + $escape_option = 'noescape'; + } + $m[1] = $this->_replaceVar($m[1]); + switch($escape_option){ + case 'auto': + return "config->autoescape === 'on' ? htmlspecialchars({$m[1]}, ENT_COMPAT, 'UTF-8', false) : {$m[1]}) ?>"; + case 'autoescape': + return ""; + case 'escape': + return ""; + case 'noescape': + return ""; + } } - return '_replaceVar($m[1]) . ' ?>'; } if($m[3]) { @@ -541,6 +567,17 @@ function _parseResource($m) { if($metafile) $result = "" . $result; return $result; + // + case 'config': + $result = ''; + if(preg_match_all('@ (\w+)="([^"]+)"@', $m[6], $config_matches, PREG_SET_ORDER)) + { + foreach($config_matches as $config_match) + { + $result .= "\$this->config->{$config_match[1]} = '" . trim(strtolower($config_match[2])) . "';"; + } + } + return ""; } } diff --git a/modules/admin/admin.admin.controller.php b/modules/admin/admin.admin.controller.php index cf12457b..2bfe7c3e 100644 --- a/modules/admin/admin.admin.controller.php +++ b/modules/admin/admin.admin.controller.php @@ -272,7 +272,7 @@ function cleanFavorite() { */ function procAdminUpdateConfig() { $adminTitle = Context::get('adminTitle'); - $file = $_FILES['adminLogo']; + $file = Context::get('adminLogo'); $oModuleModel = &getModel('module'); $oAdminConfig = $oModuleModel->getModuleConfig('admin'); diff --git a/modules/file/file.controller.php b/modules/file/file.controller.php index dc8a81d7..f30a89f6 100644 --- a/modules/file/file.controller.php +++ b/modules/file/file.controller.php @@ -26,7 +26,7 @@ function init() { **/ function procFileUpload() { Context::setRequestMethod('JSON'); - $file_info = $_FILES['Filedata']; + $file_info = Context::get('Filedata'); // An error appears if not a normally uploaded file if(!is_uploaded_file($file_info['tmp_name'])) exit(); diff --git a/modules/member/member.admin.controller.php b/modules/member/member.admin.controller.php index 7f46a71a..ae921e83 100644 --- a/modules/member/member.admin.controller.php +++ b/modules/member/member.admin.controller.php @@ -103,19 +103,19 @@ function procMemberAdminInsert(){ $this->add('member_srl', $args->member_srl); $this->setMessage($msg_code); - $profile_image = $_FILES['profile_image']; + $profile_image = Context::get('profile_image'); if(is_uploaded_file($profile_image['tmp_name'])){ $output = $oMemberController->insertProfileImage($args->member_srl, $profile_image['tmp_name']); if(!$output->toBool()) return $output; } - $image_mark = $_FILES['image_mark']; + $image_mark = Context::get('image_mark'); if(is_uploaded_file($image_mark['tmp_name'])){ $output = $oMemberController->insertImageMark($args->member_srl, $image_mark['tmp_name']); if(!$output->toBool()) return $output; } - $image_name = $_FILES['image_name']; + $image_name = Context::get('image_name'); if(is_uploaded_file($image_name['tmp_name'])){ $output = $oMemberController->insertImageName($args->member_srl, $image_name['tmp_name']); if(!$output->toBool()) return $output; diff --git a/modules/member/member.controller.php b/modules/member/member.controller.php index 7b6f8706..8e85a812 100644 --- a/modules/member/member.controller.php +++ b/modules/member/member.controller.php @@ -511,17 +511,17 @@ function procMemberModifyInfo(){ $output = $this->updateMember($args); if(!$output->toBool()) return $output; - $profile_image = $_FILES['profile_image']; + $profile_image = Context::get('profile_image'); if(is_uploaded_file($profile_image['tmp_name'])){ $this->insertProfileImage($args->member_srl, $profile_image['tmp_name']); } - $image_mark = $_FILES['image_mark']; + $image_mark = Context::get('image_mark'); if(is_uploaded_file($image_mark['tmp_name'])){ $this->insertImageMark($args->member_srl, $image_mark['tmp_name']); } - $image_name = $_FILES['image_name']; + $image_name = Context::get('image_name'); if(is_uploaded_file($image_name['tmp_name'])){ $this->insertImageName($args->member_srl, $image_name['tmp_name']); } @@ -623,7 +623,7 @@ function procMemberLeave(){ **/ function procMemberInsertProfileImage(){ // Check if the file is successfully uploaded - $file = $_FILES['profile_image']; + $file = Context::get('profile_image'); if(!is_uploaded_file($file['tmp_name'])) return $this->stop('msg_not_uploaded_profile_image'); // Ignore if member_srl is invalid or doesn't exist. $member_srl = Context::get('member_srl'); @@ -723,7 +723,7 @@ function insertProfileImage($member_srl, $target_file){ **/ function procMemberInsertImageName(){ // Check if the file is successfully uploaded - $file = $_FILES['image_name']; + $file = Context::get('image_name'); if(!is_uploaded_file($file['tmp_name'])) return $this->stop('msg_not_uploaded_image_name'); // Ignore if member_srl is invalid or doesn't exist. $member_srl = Context::get('member_srl'); @@ -857,7 +857,7 @@ function procMemberDeleteImageName($_memberSrl = 0){ **/ function procMemberInsertImageMark(){ // Check if the file is successfully uploaded - $file = $_FILES['image_mark']; + $file = Context::get('image_mark'); if(!is_uploaded_file($file['tmp_name'])) return $this->stop('msg_not_uploaded_image_mark'); // Ignore if member_srl is invalid or doesn't exist. $member_srl = Context::get('member_srl');