From 92c0b2a5178d5d7102a2fac85e8af0ba8940994f Mon Sep 17 00:00:00 2001 From: azett Date: Sat, 1 Oct 2022 13:33:34 +0200 Subject: [PATCH] Bufix: Checking uploaded files' extensions looked for the tmp file name, not the actual file name. Fixes #152 as well - thanks @s4n-h4xor! --- CHANGELOG.md | 2 +- admin/panels/uploader/admin.uploader.php | 219 ++++++++++++----------- 2 files changed, 112 insertions(+), 109 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 09710781..a3542053 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ - jQuery plugin: Updated jQuery (3.5.1 => 3.6) and jQueryUI (1.12.1 => 1.13.1) - Media Manager plugin shows 50 items per page, not 10 - LastComments plugin will not even attempt to delete or rebuild LastComments caches if LastComments plugin is not available ([#43](https://github.com/flatpressblog/flatpress/issues/43)) +- Comment Center config page threw errors ([#90](https://github.com/flatpressblog/flatpress/issues/90)) ## Themes - Leggero @@ -29,7 +30,6 @@ - Search page: Month names displayed in configured frontend language ([#132](https://github.com/flatpressblog/flatpress/issues/132)) ## Other bugfixes -- Comment Center config page threw errors ([#90](https://github.com/flatpressblog/flatpress/issues/90)) - Plugin management page: Removed empty warning messages box - Fixed error at prev link on first / next link on last entry ([#95](https://github.com/flatpressblog/flatpress/issues/95)) - Logout redirects to home page again ([#119](https://github.com/flatpressblog/flatpress/issues/119)) diff --git a/admin/panels/uploader/admin.uploader.php b/admin/panels/uploader/admin.uploader.php index 8cc6ca22..1fc8aec2 100755 --- a/admin/panels/uploader/admin.uploader.php +++ b/admin/panels/uploader/admin.uploader.php @@ -102,133 +102,136 @@ function onupload() { foreach ($_FILES ["upload"] ["error"] as $key => $error) { - if ($error == UPLOAD_ERR_OK) { - $tmp_name = $_FILES ["upload"] ["tmp_name"] [$key]; - $name = $_FILES ["upload"] ["name"] [$key]; + // Upload went wrong -> jump to the next file + if ($error != UPLOAD_ERR_OK) { + continue; + } + + $tmp_name = $_FILES ["upload"] ["tmp_name"] [$key]; + $name = $_FILES ["upload"] ["name"] [$key]; + + $dir = ATTACHS_DIR; + + /* + * second check extension list + * https://stackoverflow.com/questions/4166762/php-image-upload-security-check-list + * + * 2019-11-24 - laborix + */ + + $uploadfilename = strtolower($name); - $dir = ATTACHS_DIR; + $isForbidden = false; + $deeptest = array(); + $extcount = 0; + $deeptest = explode('.', $uploadfilename); + $extcount = count($deeptest); + if ($extcount == 1) { /* - * second check extension list - * https://stackoverflow.com/questions/4166762/php-image-upload-security-check-list + * none extension like .jpg or something else * - * 2019-11-24 - laborix + * possible filename = simple-file-without-extension - linux like ok */ - - $uploadfilename = strtolower($tmp_name); - $isForbidden = false; - $deeptest = array(); - $extcount = 0; - $deeptest = explode('.', $uploadfilename); - $extcount = count($deeptest); - - if ($extcount == 1) { - /* - * none extension like .jpg or something else - * - * possible filename = simple-file-without-extension - linux like ok - */ - $isForbidden = false; - } elseif ($extcount == 2) { - /* - * Only one possible extension - * - * possible filename = 1.jpg - * possible filename = admin.uploader.php - * possible filename = .htaccess - * and so on... - */ - $check_ext1 = ""; - $check_ext1 = trim($deeptest [1], "\x00..\x1F"); - if (in_array($check_ext1, $blacklist_extensions)) { - $isForbidden = true; - } else { - $isForbidden = false; - } - } elseif ($extcount > 2) { - /* - * Chekc only the last two possible extensions - * - * Hint: OWASP - Unrestricted File Upload - * - * In Apache, a php file might be executed using the - * double extension technique such as "file.php.jpg" - * when ".jpg" is allowed. - * - * possible filename = 1.PhP.jpg - * possible filename = admin.uploader.php.JPg - * and so on... - */ - $check_ext1 = ""; - $check_ext2 = ""; - $check_ext1 = trim($deeptest [$extcount - 1], "\x00..\x1F"); - if (in_array($check_ext1, $blacklist_extensions)) { - $isForbidden = true; - } else { - $isForbidden = false; - } - /* Test only if first extension check are not in the blacklist */ - if (!$isForbidden) { - $check_ext2 = trim($deeptest [$extcount - 2], "\x00..\x1F"); - if (in_array($check_ext2, $blacklist_extensions)) { - $isForbidden = true; - } else { - $isForbidden = false; - } - } - } + } elseif ($extcount == 2) { /* - * If one blacklisted extension found then - * return with -1 = An error occurred while trying to upload. + * Only one possible extension + * + * possible filename = 1.jpg + * possible filename = admin.uploader.php + * possible filename = .htaccess + * and so on... */ - if ($isForbidden) { - $this->smarty->assign('success', $success ? 1 : -1); - sess_add('admin_uploader_files', $uploaded_files); - return -1; + $check_ext1 = ""; + $check_ext1 = trim($deeptest [1], "\x00..\x1F"); + if (in_array($check_ext1, $blacklist_extensions)) { + $isForbidden = true; + } else { + $isForbidden = false; } - + } elseif ($extcount > 2) { /* - * third check extension - * if someone upload a .php file as .gif, .jpg or .txt - * if someone upload a .html file as .gif, .jpg or .txt + * Chekc only the last two possible extensions + * + * Hint: OWASP - Unrestricted File Upload + * + * In Apache, a php file might be executed using the + * double extension technique such as "file.php.jpg" + * when ".jpg" is allowed. * - * 2019-11-24 - laborix + * possible filename = 1.PhP.jpg + * possible filename = admin.uploader.php.JPg + * and so on... */ - - if (version_compare(PHP_VERSION, '5.3.0') < 0) - return -1; - if (!function_exists('finfo_open')) - return -1; - - $finfo = finfo_open(FILEINFO_MIME_TYPE); - $mime = finfo_file($finfo, $tmp_name); - finfo_close($finfo); - - if (($mime == "text/x-php") || ($mime == "text/html")) { - $this->smarty->assign('success', $success ? 1 : -1); - sess_add('admin_uploader_files', $uploaded_files); - return -1; + $check_ext1 = ""; + $check_ext2 = ""; + $check_ext1 = trim($deeptest [$extcount - 1], "\x00..\x1F"); + if (in_array($check_ext1, $blacklist_extensions)) { + $isForbidden = true; + } else { + $isForbidden = false; } + /* Test only if first extension check are not in the blacklist */ + if (!$isForbidden) { + $check_ext2 = trim($deeptest [$extcount - 2], "\x00..\x1F"); + if (in_array($check_ext2, $blacklist_extensions)) { + $isForbidden = true; + } else { + $isForbidden = false; + } + } + } + /* + * If one blacklisted extension found then + * return with -1 = An error occurred while trying to upload. + */ + if ($isForbidden) { + $this->smarty->assign('success', $success ? 1 : -1); + sess_add('admin_uploader_files', $uploaded_files); + return -1; + } - $ext = strtolower(strrchr($name, '.')); + /* + * third check extension + * if someone upload a .php file as .gif, .jpg or .txt + * if someone upload a .html file as .gif, .jpg or .txt + * + * 2019-11-24 - laborix + */ + + if (version_compare(PHP_VERSION, '5.3.0') < 0) + return -1; + if (!function_exists('finfo_open')) + return -1; + + $finfo = finfo_open(FILEINFO_MIME_TYPE); + $mime = finfo_file($finfo, $tmp_name); + finfo_close($finfo); + + if (($mime == "text/x-php") || ($mime == "text/html")) { + $this->smarty->assign('success', $success ? 1 : -1); + sess_add('admin_uploader_files', $uploaded_files); + return -1; + } - if (in_array($ext, $imgs)) { - $dir = IMAGES_DIR; - } + $ext = strtolower(strrchr($name, '.')); + + if (in_array($ext, $imgs)) { + $dir = IMAGES_DIR; + } - $name = sanitize_title(substr($name, 0, -strlen($ext))) . $ext; + $name = sanitize_title(substr($name, 0, -strlen($ext))) . $ext; - $target = "$dir/$name"; - @umask(022); - $success = move_uploaded_file($tmp_name, $target); - @chmod($target, 0766); + $target = "$dir/$name"; + @umask(022); + $success = move_uploaded_file($tmp_name, $target); + @chmod($target, 0766); - $uploaded_files [] = $name; + $uploaded_files [] = $name; - // one failure will make $success == false :) - $success &= $success; - } + // one failure will make $success == false :) + $success &= $success; } if ($uploaded_files) {