From 18ec8078eb0ee7c429ae170d5ce59872c9f257b4 Mon Sep 17 00:00:00 2001 From: bnu Date: Wed, 20 Mar 2019 12:51:13 +0900 Subject: [PATCH] XEVE-19-001, XEVE-19-003, XEVE-19-006 --- .htaccess | 2 +- classes/context/Context.class.php | 4 ++- classes/extravar/Extravar.class.php | 14 +++++----- classes/security/UploadFileFilter.class.php | 31 +++++++++++++++++++-- config/func.inc.php | 4 +-- modules/comment/comment.controller.php | 4 +-- modules/comment/comment.item.php | 10 +++---- modules/document/document.controller.php | 4 +-- modules/document/document.item.php | 2 +- modules/file/file.controller.php | 20 ++++++++++--- modules/member/member.model.php | 6 ++-- modules/module/module.controller.php | 14 ++++++++-- widgets/mcontent/mcontent.class.php | 2 +- 13 files changed, 83 insertions(+), 34 deletions(-) diff --git a/.htaccess b/.htaccess index f619de5b37..1846b7a2bf 100644 --- a/.htaccess +++ b/.htaccess @@ -3,7 +3,7 @@ RewriteEngine On # deny access to files that may contain sensitive information RewriteRule ^(.*/)?\.(editor|git|ht|jshint|travis) - [L,F] RewriteRule ^(codeception(.*)\.yml|composer(.*)\.(json|lock)|package\.json)$ - [L,F] -RewriteRule ^files/(attach|config|cache/store)/.+\.php$ - [L,F] +RewriteRule ^files/(attach|config|cache/store)/.+\.(ph(p|t|ar)?[0-9]?|p?html?|cgi|pl|exe|(a|j)sp|inc)$ - [L,F] RewriteRule ^files/(env|member_extra_info/(new_message_flags|point))/ - [L,F] # reserve XE Layout Template Source File (*.html) diff --git a/classes/context/Context.class.php b/classes/context/Context.class.php index 0c9962f6ae..41510dc58d 100644 --- a/classes/context/Context.class.php +++ b/classes/context/Context.class.php @@ -1493,13 +1493,15 @@ function _setUploadedArgument() foreach($_FILES as $key => $val) { $tmp_name = $val['tmp_name']; + if(!is_array($tmp_name)) { if(!UploadFileFilter::check($tmp_name, $val['name'])) { + unset($_FILES[$key]); continue; } - $val['name'] = htmlspecialchars($val['name'], ENT_COMPAT | ENT_HTML401, 'UTF-8', FALSE); + $val['name'] = escape($val['name'], FALSE); $this->set($key, $val, TRUE); $this->is_uploaded = TRUE; } diff --git a/classes/extravar/Extravar.class.php b/classes/extravar/Extravar.class.php index bdf9404981..e9337e3b2a 100644 --- a/classes/extravar/Extravar.class.php +++ b/classes/extravar/Extravar.class.php @@ -209,7 +209,7 @@ function _getTypeValue($type, $value) { $value = 'http://' . $value; } - return htmlspecialchars($value, ENT_COMPAT | ENT_HTML401, 'UTF-8', false); + return escpe($value, false); case 'tel' : if(is_array($value)) @@ -228,7 +228,7 @@ function _getTypeValue($type, $value) $values = array_values($values); for($i = 0, $c = count($values); $i < $c; $i++) { - $values[$i] = trim(htmlspecialchars($values[$i], ENT_COMPAT | ENT_HTML401, 'UTF-8', false)); + $values[$i] = trim(escpe($values[$i], false)); } return $values; @@ -255,7 +255,7 @@ function _getTypeValue($type, $value) $values = array_values($values); for($i = 0, $c = count($values); $i < $c; $i++) { - $values[$i] = trim(htmlspecialchars($values[$i], ENT_COMPAT | ENT_HTML401, 'UTF-8', false)); + $values[$i] = trim(escape($values[$i], false)); } return $values; @@ -276,7 +276,7 @@ function _getTypeValue($type, $value) $values = array_values($values); for($i = 0, $c = count($values); $i < $c; $i++) { - $values[$i] = trim(htmlspecialchars($values[$i], ENT_COMPAT | ENT_HTML401, 'UTF-8', false)); + $values[$i] = trim(escape($values[$i], false)); } return $values; @@ -285,7 +285,7 @@ function _getTypeValue($type, $value) //case 'text' : //case 'textarea' : default : - return htmlspecialchars($value, ENT_COMPAT | ENT_HTML401, 'UTF-8', false); + return escape($value, false); } } @@ -311,10 +311,10 @@ function getValueHTML() switch($this->type) { case 'homepage' : - return ($value) ? (sprintf('%s', $value, strlen($value) > 60 ? substr($value, 0, 40) . '...' . substr($value, -10) : $value)) : ""; + return ($value) ? (sprintf('%s', escape($value, false), strlen($value) > 60 ? substr($value, 0, 40) . '...' . substr($value, -10) : $value)) : ""; case 'email_address' : - return ($value) ? sprintf('%s', $value, $value) : ""; + return ($value) ? sprintf('%s', escape($value, false), $value) : ""; case 'tel' : return sprintf('%s-%s-%s', $value[0], $value[1], $value[2]); diff --git a/classes/security/UploadFileFilter.class.php b/classes/security/UploadFileFilter.class.php index 5bdd9f5f3d..379cba1c66 100644 --- a/classes/security/UploadFileFilter.class.php +++ b/classes/security/UploadFileFilter.class.php @@ -28,11 +28,12 @@ public static function check($file, $filename = null) // Get the extension. $ext = $filename ? strtolower(substr(strrchr($filename, '.'), 1)) : ''; + $mimetype = self::_getMimetype($file, true); // 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); + $is_xml = preg_match('/<(?:\?xml|svg)\b/i', $first4kb); // Check SVG files. if (($ext === 'svg' || $is_xml) && !self::_checkSVG($fp, 0, $filesize)) @@ -41,6 +42,18 @@ public static function check($file, $filename = null) return false; } + if (in_array($ext, array('jpg', 'jpeg', 'png', 'gif')) && $mimetype !== 'image') + { + return false; + } + + if (preg_match("/(wm[va]|mpe?g|avi|flv|mp[1-4]|as[fx]|wav|midi?|moo?v|qt|r[am]{1,2}|m4v)$/i", $file)) + { + if ($mimetype !== 'video' && $mimetype !== 'audio') { + return false; + } + } + // Check XML files. if (($ext === 'xml' || $is_xml) && !self::_checkXML($fp, 0, $filesize)) { @@ -49,7 +62,7 @@ public static function check($file, $filename = null) } // Check HTML files. - if (($ext === 'html' || $ext === 'shtml' || $ext === 'xhtml' || $ext === 'phtml' || $is_xml) && !self::_checkHTML($fp, 0, $filesize)) + if (($ext === 'html' || $ext === 'shtml' || $ext === 'xhtml' || $ext === 'phtml') && !$is_xml && !self::_checkHTML($fp, 0, $filesize)) { fclose($fp); return false; @@ -147,6 +160,20 @@ protected static function _matchStream($regexp, $fp, $from, $to, $block_size = 1 } return false; } + + protected static function _getMimetype($file, $trim_subtype = false) + { + $finfo = finfo_open(FILEINFO_MIME_TYPE); + $mime_type = finfo_file($finfo, $file); + finfo_close($finfo); + + if($trim_subtype) + { + $mime_type = strstr($mime_type, '/', true); + } + + return $mime_type; + } } /* End of file : UploadFileFilter.class.php */ /* Location: ./classes/security/UploadFileFilter.class.php */ diff --git a/config/func.inc.php b/config/func.inc.php index 69e15fb795..8c9eba1ed3 100644 --- a/config/func.inc.php +++ b/config/func.inc.php @@ -1162,10 +1162,10 @@ function blockWidgetCode($content) * @param string $file Taget file path * @return bool */ -function checkUploadedFile($file) +function checkUploadedFile($file, $filename = null) { require_once(_XE_PATH_ . 'classes/security/UploadFileFilter.class.php'); - return UploadFileFilter::check($file); + return UploadFileFilter::check($file, $filename); } /** diff --git a/modules/comment/comment.controller.php b/modules/comment/comment.controller.php index c000f4a238..a45e2f5605 100644 --- a/modules/comment/comment.controller.php +++ b/modules/comment/comment.controller.php @@ -279,7 +279,7 @@ function insertComment($obj, $manual_inserted = FALSE) if($obj->homepage) { - $obj->homepage = removeHackTag($obj->homepage); + $obj->homepage = escape($obj->homepage, false); if(!preg_match('/^[a-z]+:\/\//i',$obj->homepage)) { $obj->homepage = 'http://'.$obj->homepage; @@ -692,7 +692,7 @@ function updateComment($obj, $is_admin = FALSE, $manual_updated = FALSE) if($obj->homepage) { - $obj->homepage = removeHackTag($obj->homepage); + $obj->homepage = escape($obj->homepage); if(!preg_match('/^[a-z]+:\/\//i',$obj->homepage)) { $obj->homepage = 'http://'.$obj->homepage; diff --git a/modules/comment/comment.item.php b/modules/comment/comment.item.php index 233f8c1c6d..228e27e54d 100644 --- a/modules/comment/comment.item.php +++ b/modules/comment/comment.item.php @@ -255,7 +255,7 @@ function getHomepageUrl() $url = "http://" . $url; } - return htmlspecialchars($url, ENT_COMPAT | ENT_HTML401, 'UTF-8', false); + return escape($url, false); } function getMemberSrl() @@ -265,17 +265,17 @@ function getMemberSrl() function getUserID() { - return htmlspecialchars($this->get('user_id'), ENT_COMPAT | ENT_HTML401, 'UTF-8', false); + return escape($this->get('user_id'), false); } function getUserName() { - return htmlspecialchars($this->get('user_name'), ENT_COMPAT | ENT_HTML401, 'UTF-8', false); + return escape($this->get('user_name'), false); } function getNickName() { - return htmlspecialchars($this->get('nick_name'), ENT_COMPAT | ENT_HTML401, 'UTF-8', false); + return escape($this->get('nick_name'), false); } /** @@ -296,7 +296,7 @@ function getContentText($strlen = 0) return cut_str(strip_tags($content), $strlen, '...'); } - return htmlspecialchars($content, ENT_COMPAT | ENT_HTML401, 'UTF-8', false); + return escape($content, false); } /** diff --git a/modules/document/document.controller.php b/modules/document/document.controller.php index 5f3b5e9ec5..7cedd2f7e5 100644 --- a/modules/document/document.controller.php +++ b/modules/document/document.controller.php @@ -208,7 +208,7 @@ function insertDocument($obj, $manual_inserted = false, $isRestore = false, $isL if($obj->allow_trackback!='Y') $obj->allow_trackback = 'N'; if($obj->homepage) { - $obj->homepage = removeHackTag($obj->homepage); + $obj->homepage = escape($obj->homepage); if(!preg_match('/^[a-z]+:\/\//i',$obj->homepage)) { $obj->homepage = 'http://'.$obj->homepage; @@ -414,7 +414,7 @@ function updateDocument($source_obj, $obj, $manual_updated = FALSE) if($obj->allow_trackback!='Y') $obj->allow_trackback = 'N'; if($obj->homepage) { - $obj->homepage = removeHackTag($obj->homepage); + $obj->homepage = escape($obj->homepage); if(!preg_match('/^[a-z]+:\/\//i',$obj->homepage)) { $obj->homepage = 'http://'.$obj->homepage; diff --git a/modules/document/document.item.php b/modules/document/document.item.php index fcf0e094a7..4201422e0a 100644 --- a/modules/document/document.item.php +++ b/modules/document/document.item.php @@ -345,7 +345,7 @@ function getHomepageUrl() if(strncasecmp('http://', $url, 7) !== 0 && strncasecmp('https://', $url, 8) !== 0) $url = 'http://' . $url; - return $url; + return escape($url, false); } function getMemberSrl() diff --git a/modules/file/file.controller.php b/modules/file/file.controller.php index 3582e27c84..4cec0d58de 100644 --- a/modules/file/file.controller.php +++ b/modules/file/file.controller.php @@ -50,9 +50,21 @@ function procFileUpload() $this->add('file_size',$output->get('file_size')); $this->add('direct_download',$output->get('direct_download')); $this->add('source_filename',$output->get('source_filename')); - $this->add('download_url',$output->get('uploaded_filename')); $this->add('upload_target_srl',$output->get('upload_target_srl')); - if($output->error != '0') $this->stop($output->message); + $this->add('download_url',$output->get('uploaded_filename')); + + if($output->get('direct_download') === 'Y') + { + $this->add('download_url',$output->get('uploaded_filename')); + } + else + { + $this->add('download_url',$oFileModel->getDownloadUrl($output->get('file_srl'), $output->get('sid'), $module_srl)); + } + + if($output->error != '0') { + $this->stop($output->message); + } } /** @@ -709,7 +721,7 @@ function insertFile($file_info, $module_srl, $upload_target_srl, $download_count } // https://github.com/xpressengine/xe-core/issues/1713 - $file_info['name'] = preg_replace('/\.(php|phtm|phar|html?|cgi|pl|exe|jsp|asp|inc)/i', '$0-x',$file_info['name']); + $file_info['name'] = preg_replace('/\.((ph(p|t|ar)?[0-9]?|p?html?|cgi|pl|exe|(?:a|j)sp|inc).*)$/i', '$0-x',$file_info['name']); $file_info['name'] = removeHackTag($file_info['name']); $file_info['name'] = str_replace(array('<','>'),array('%3C','%3E'),$file_info['name']); $file_info['name'] = str_replace('&', '&', $file_info['name']); @@ -746,7 +758,7 @@ function insertFile($file_info, $module_srl, $upload_target_srl, $download_count if(!FileHandler::makeDir($path)) return new BaseObject(-1,'msg_not_permitted_create'); // Check uploaded file - if(!$manual_insert && !checkUploadedFile($file_info['tmp_name'])) return new BaseObject(-1,'msg_file_upload_error'); + if(!$manual_insert && !checkUploadedFile($file_info['tmp_name'], $file_info['name'])) return new BaseObject(-1,'msg_file_upload_error'); // Get random number generator $random = new Password(); diff --git a/modules/member/member.model.php b/modules/member/member.model.php index aaedcfd495..ccaf5cc4d2 100644 --- a/modules/member/member.model.php +++ b/modules/member/member.model.php @@ -156,16 +156,16 @@ function getMemberMenu() // Send an email only if email address is public if(($logged_info->is_admin == 'Y' || $email_config->isPublic == 'Y') && $member_info->email_address) { - $url = 'mailto:'.htmlspecialchars($member_info->email_address, ENT_COMPAT | ENT_HTML401, 'UTF-8', false); + $url = 'mailto:'.escape($member_info->email_address, false); $oMemberController->addMemberPopupMenu($url,'cmd_send_email',$icon_path); } } // View homepage info if($member_info->homepage) - $oMemberController->addMemberPopupMenu(htmlspecialchars($member_info->homepage, ENT_COMPAT | ENT_HTML401, 'UTF-8', false), 'homepage', '', 'blank'); + $oMemberController->addMemberPopupMenu(escape($member_info->homepage, false), 'homepage', '', 'blank'); // View blog info if($member_info->blog) - $oMemberController->addMemberPopupMenu(htmlspecialchars($member_info->blog, ENT_COMPAT | ENT_HTML401, 'UTF-8', false), 'blog', '', 'blank'); + $oMemberController->addMemberPopupMenu(escape($member_info->blog, false), 'blog', '', 'blank'); // Call a trigger (after) ModuleHandler::triggerCall('member.getMemberMenu', 'after', $null); // Display a menu for editting member info to a top administrator diff --git a/modules/module/module.controller.php b/modules/module/module.controller.php index 741a9768c1..0a30f320f4 100644 --- a/modules/module/module.controller.php +++ b/modules/module/module.controller.php @@ -1276,7 +1276,11 @@ function updateModuleFileBox($vars) $path = $oModuleModel->getModuleFileBoxPath($vars->module_filebox_srl); FileHandler::makeDir($path); - $save_filename = sprintf('%s%s.%s',$path, $vars->module_filebox_srl, $ext); + $random = new Password(); + $ext = explode($vars->addfile['name'], '.'); + $ext = strtolower(array_pop($ext)); + + $save_filename = sprintf('%s%s.%s', $path, $random->createSecureSalt(32, 'hex'), $ext); $tmp = $vars->addfile['tmp_name']; // Check uploaded file @@ -1287,7 +1291,7 @@ function updateModuleFileBox($vars) return false; } - $args->fileextension = strtolower(substr(strrchr($vars->addfile['name'],'.'),1)); + $args->fileextension = $ext; $args->filename = $save_filename; $args->filesize = $vars->addfile['size']; } @@ -1313,7 +1317,11 @@ function insertModuleFileBox($vars) $oModuleModel = getModel('module'); $path = $oModuleModel->getModuleFileBoxPath($vars->module_filebox_srl); FileHandler::makeDir($path); - $save_filename = sprintf('%s%s.%s',$path, $vars->module_filebox_srl, $vars->ext); + $random = new Password(); + $ext = explode($vars->addfile['name'], '.'); + $ext = strtolower(array_pop($ext)); + + $save_filename = sprintf('%s%s.%s',$path, $random->createSecureSalt(32, 'hex'), $vars->ext); $tmp = $vars->addfile['tmp_name']; // Check uploaded file diff --git a/widgets/mcontent/mcontent.class.php b/widgets/mcontent/mcontent.class.php index 376229bc45..708f36f224 100644 --- a/widgets/mcontent/mcontent.class.php +++ b/widgets/mcontent/mcontent.class.php @@ -754,7 +754,7 @@ function getFirstThumbnailIdx() function getLink() { - return $this->get('url'); + return escape($this->get('url'), false); } function getModuleSrl() {