From 86f9693dc64ed54220ed6c10e13e824ca4f6aacf Mon Sep 17 00:00:00 2001 From: JediKev Date: Wed, 8 Mar 2023 10:31:12 -0600 Subject: [PATCH] xss: Draft Files This mitigates an XSS vulnerability with files uploaded through drafts. --- bootstrap.php | 9 ++++ include/ajax.draft.php | 114 +++++++++++++++++++--------------------- include/class.file.php | 23 +++++--- include/class.forms.php | 4 ++ 4 files changed, 84 insertions(+), 66 deletions(-) diff --git a/bootstrap.php b/bootstrap.php index b0baf0c7e1..a6dba4b4ec 100644 --- a/bootstrap.php +++ b/bootstrap.php @@ -54,6 +54,15 @@ function exif_imagetype ($filename) { } } + if (!function_exists('exif_imagetype')) { + function exif_imagetype ($filename) { + if ((list($width,$height,$type,) = getimagesize($filename)) !== false) + return $type; + + return false; + } + } + if (!isset($_SERVER['REMOTE_ADDR'])) $_SERVER['REMOTE_ADDR'] = ''; } diff --git a/include/ajax.draft.php b/include/ajax.draft.php index 8d7ab5e2c8..6d1b847a0d 100644 --- a/include/ajax.draft.php +++ b/include/ajax.draft.php @@ -53,78 +53,72 @@ static function _updateDraft($draft) { static function _uploadInlineImage($draft) { global $cfg; - if (!isset($_POST['data']) && !isset($_FILES['file'])) + if (!isset($_FILES['file'])) Http::response(422, "File not included properly"); # Fixup for expected multiple attachments - if (isset($_FILES['file'])) { - $file = AttachmentFile::format($_FILES['file']); - - # Allow for data-uri uploaded files - $fp = fopen($file[0]['tmp_name'], 'rb'); - if (fread($fp, 5) == 'data:') { - $data = 'data:'; - while ($block = fread($fp, 8192)) - $data .= $block; - $file[0] = Format::parseRfc2397($data); - list(,$ext) = explode('/', $file[0]['type'], 2); - $file[0] += array( - 'name' => Misc::randCode(8).'.'.$ext, - 'size' => strlen($file[0]['data']), - ); - } - fclose($fp); + $file = AttachmentFile::format($_FILES['file']); + + # Allow for data-uri uploaded files + $fp = fopen($file[0]['tmp_name'], 'rb'); + if (fread($fp, 5) == 'data:') { + $data = 'data:'; + while ($block = fread($fp, 8192)) + $data .= $block; + $file[0] = Format::parseRfc2397($data); + list(,$ext) = explode('/', $file[0]['type'], 2); + $file[0] += array( + 'name' => Misc::randCode(8).'.'.$ext, + 'size' => strlen($file[0]['data']), + ); + } + fclose($fp); + + // Check file type to ensure image + $type = $file[0]['type']; + if (strpos($file[0]['type'], 'image/') !== 0) + return Http::response(403, + JsonDataEncoder::encode(array( + 'error' => 'File type is not allowed', + )) + ); - # TODO: Detect unacceptable attachment extension - # TODO: Verify content-type and check file-content to ensure image - $type = $file[0]['type']; - if (strpos($file[0]['type'], 'image/') !== 0) - return Http::response(403, - JsonDataEncoder::encode(array( - 'error' => 'File type is not allowed', - )) - ); + // Check if file is truly an image + if (!FileUploadField::isValidFile($file[0])) + return Http::response(403, + JsonDataEncoder::encode(array( + 'error' => 'File is not valid', + )) + ); - # TODO: Verify file size is acceptable - if ($file[0]['size'] > $cfg->getMaxFileSize()) + // Verify file size is acceptable + if ($file[0]['size'] > $cfg->getMaxFileSize()) + return Http::response(403, + JsonDataEncoder::encode(array( + 'error' => 'File is too large', + )) + ); + + // Paste uploads in Chrome will have a name of 'blob' + if ($file[0]['name'] == 'blob') + $file[0]['name'] = 'screenshot-'.Misc::randCode(4); + + $ids = $draft->attachments->upload($file); + + if (!$ids) { + if ($file[0]['error']) { return Http::response(403, JsonDataEncoder::encode(array( - 'error' => 'File is too large', + 'error' => $file[0]['error'], )) ); - // Paste uploads in Chrome will have a name of 'blob' - if ($file[0]['name'] == 'blob') - $file[0]['name'] = 'screenshot-'.Misc::randCode(4); - - $ids = $draft->attachments->upload($file); - - if (!$ids) { - if ($file[0]['error']) { - return Http::response(403, - JsonDataEncoder::encode(array( - 'error' => $file[0]['error'], - )) - ); - } - else - return Http::response(500, 'Unable to attach image'); } - - $id = (is_array($ids)) ? $ids[0] : $ids; - } - else { - $type = explode('/', $_POST['contentType']); - $info = array( - 'data' => base64_decode($_POST['data']), - 'name' => Misc::randCode(10).'.'.$type[1], - // TODO: Ensure _POST['contentType'] - 'type' => $_POST['contentType'], - ); - // TODO: Detect unacceptable filetype - // TODO: Verify content-type and check file-content to ensure image - $id = $draft->attachments->save($info); + else + return Http::response(500, 'Unable to attach image'); } + + $id = (is_array($ids)) ? $ids[0] : $ids; if (!($f = AttachmentFile::lookup($id))) return Http::response(500, 'Unable to attach image'); diff --git a/include/class.file.php b/include/class.file.php index ddc3807900..d7c8207d09 100644 --- a/include/class.file.php +++ b/include/class.file.php @@ -1054,12 +1054,8 @@ function setMimeType($type) { } function getMimeType() { - if (!isset($this->_mimetype)) { - // Try to to auto-detect mime type - $finfo = new finfo(FILEINFO_MIME); - $this->_mimetype = $finfo->buffer($this->getContents(), - FILEINFO_MIME_TYPE); - } + if (!isset($this->_mimetype)) + $this->_mimetype = self::mime_type($this->getRealPath()); return $this->_mimetype; } @@ -1075,6 +1071,21 @@ function getContents() { function getData() { return $this->getContents(); } + + /* + * Given a filepath - auto detect the mime type + * + */ + static function mime_type($filepath) { + // Try to to auto-detect mime type + $type = null; + if (function_exists('finfo_open')) { + $finfo = finfo_open(FILEINFO_MIME_TYPE); + $type = finfo_file($finfo, $filepath); + finfo_close($finfo); + } + return $type ?: mime_content_type($filepath); + } } ?> diff --git a/include/class.forms.php b/include/class.forms.php index f8166bb62d..0c042eeb37 100644 --- a/include/class.forms.php +++ b/include/class.forms.php @@ -3911,6 +3911,10 @@ function uploadAttachment(&$file) { } static function isValidFile($file) { + // Make sure mime type is valid + if (strcasecmp(FileObject::mime_type($file['tmp_name']), + $file['type']) !== 0) + return false; // Check invalid image hacks if ($file['tmp_name']