From eff61f9351984e018cc055fd1ba567fa9dec3869 Mon Sep 17 00:00:00 2001 From: kiat ng Date: Fri, 30 Sep 2022 20:44:30 +0800 Subject: [PATCH 1/5] Fixed issue #475. --- .../core/Mage/Core/Model/File/Validator/Image.php | 13 +++++++++---- app/code/core/Mage/Core/etc/config.xml | 5 +---- app/code/core/Mage/Core/etc/system.xml | 10 ++++++++++ app/locale/en_US/Mage_Core.csv | 2 ++ 4 files changed, 22 insertions(+), 8 deletions(-) diff --git a/app/code/core/Mage/Core/Model/File/Validator/Image.php b/app/code/core/Mage/Core/Model/File/Validator/Image.php index b0238e9e4de..3cffb63c96a 100644 --- a/app/code/core/Mage/Core/Model/File/Validator/Image.php +++ b/app/code/core/Mage/Core/Model/File/Validator/Image.php @@ -74,7 +74,7 @@ public function setAllowedImageTypes(array $imageFileExtensions = []) } /** - * Validation callback for checking is file is image + * Validation callback for checking if file is image * * @param string $filePath Path to temporary uploaded file * @return null @@ -85,8 +85,13 @@ public function validate($filePath) list($imageWidth, $imageHeight, $fileType) = getimagesize($filePath); if ($fileType) { if ($this->isImageType($fileType)) { - /** if 'general/reprocess_images/active' false then skip image reprocessing. */ - if (!Mage::getStoreConfigFlag('general/reprocess_images/active')) { + /** Check deprecated 'general/reprocess_images/active' for BC. If false then skip image reprocessing. */ + if (Mage::getStoreConfig('general/reprocess_images/active') !== null) { + $imageQuality = Mage::getStoreConfigFlag('general/reprocess_images/active') ? 100 : 0; + } else { + $imageQuality = (int) Mage::getStoreConfig('admin/security/reprocess_image_quality'); + } + if ($imageQuality === 0) { return null; } //replace tmp image with re-sampled copy to exclude images with malicious data @@ -116,7 +121,7 @@ public function validate($filePath) imagegif($img, $filePath); break; case IMAGETYPE_JPEG: - imagejpeg($img, $filePath, 100); + imagejpeg($img, $filePath, $imageQuality); break; case IMAGETYPE_PNG: imagepng($img, $filePath); diff --git a/app/code/core/Mage/Core/etc/config.xml b/app/code/core/Mage/Core/etc/config.xml index 5176685c5fb..643f0e89a01 100644 --- a/app/code/core/Mage/Core/etc/config.xml +++ b/app/code/core/Mage/Core/etc/config.xml @@ -440,6 +440,7 @@ 1 10800 0 + 85 @@ -497,10 +498,6 @@ - - - 1 - diff --git a/app/code/core/Mage/Core/etc/system.xml b/app/code/core/Mage/Core/etc/system.xml index 5d41b21654f..24b4e70945e 100644 --- a/app/code/core/Mage/Core/etc/system.xml +++ b/app/code/core/Mage/Core/etc/system.xml @@ -1280,6 +1280,16 @@ 0 0 + + + Optimum value is 85, higher value will increase file size. Set to 0 to turn off images reprocessing, which may cause security risks. + text + required-entry validate-digits validate-digits-range digits-range-0-100 + 180 + 1 + 1 + 1 + diff --git a/app/locale/en_US/Mage_Core.csv b/app/locale/en_US/Mage_Core.csv index da7783a717b..924034de292 100644 --- a/app/locale/en_US/Mage_Core.csv +++ b/app/locale/en_US/Mage_Core.csv @@ -167,6 +167,7 @@ "How many links to display at once.","How many links to display at once." "ID Path for Specified Store","ID Path for Specified Store" "If the current frame position does not cover utmost pages, will render link to current position plus/minus this value.","If the current frame position does not cover utmost pages, will render link to current position plus/minus this value." +"Image Reprocess Quality","Image Reprocess Quality" "Incorrect credit card expiration date.","Incorrect credit card expiration date." "Input type ""%value%"" not found in the input types list.","Input type ""%value%"" not found in the input types list." "Invalid MIME type.","Invalid MIME type." @@ -226,6 +227,7 @@ "New Website","New Website" "No","No" "Offloader header","Offloader header" +"Optimum value is 85, higher value will increase file size. Set to 0 to turn off images reprocessing, which may cause security risks.","Optimum value is 85, higher value will increase file size. Set to 0 to turn off images reprocessing, which may cause security risks." "PHP SOAP extension is required.","PHP SOAP extension is required." "Package","Package" "Pagination","Pagination" From 2df1069edf09151ce0d1de7713fc7229fe541bef Mon Sep 17 00:00:00 2001 From: kiat ng Date: Sat, 1 Oct 2022 00:23:32 +0800 Subject: [PATCH 2/5] Removed text as it is the default. --- app/code/core/Mage/Core/etc/system.xml | 1 - 1 file changed, 1 deletion(-) diff --git a/app/code/core/Mage/Core/etc/system.xml b/app/code/core/Mage/Core/etc/system.xml index 24b4e70945e..53411e945bc 100644 --- a/app/code/core/Mage/Core/etc/system.xml +++ b/app/code/core/Mage/Core/etc/system.xml @@ -1283,7 +1283,6 @@ Optimum value is 85, higher value will increase file size. Set to 0 to turn off images reprocessing, which may cause security risks. - text required-entry validate-digits validate-digits-range digits-range-0-100 180 1 From 12071f7610f9a32861fb89d3c2a87f9d793c4900 Mon Sep 17 00:00:00 2001 From: kiat ng Date: Sat, 1 Oct 2022 19:54:17 +0800 Subject: [PATCH 3/5] Fixed bug on users not able to modify image quality in backend if deprecated config exists. --- .../core/Mage/Core/Model/File/Validator/Image.php | 13 +++++++++---- app/code/core/Mage/Core/etc/config.xml | 1 - app/code/core/Mage/Core/etc/system.xml | 4 ++-- app/locale/en_US/Mage_Core.csv | 2 +- 4 files changed, 12 insertions(+), 8 deletions(-) diff --git a/app/code/core/Mage/Core/Model/File/Validator/Image.php b/app/code/core/Mage/Core/Model/File/Validator/Image.php index 3cffb63c96a..215718c265b 100644 --- a/app/code/core/Mage/Core/Model/File/Validator/Image.php +++ b/app/code/core/Mage/Core/Model/File/Validator/Image.php @@ -75,6 +75,7 @@ public function setAllowedImageTypes(array $imageFileExtensions = []) /** * Validation callback for checking if file is image + * Destroy malicious code in image by reprocessing * * @param string $filePath Path to temporary uploaded file * @return null @@ -85,11 +86,15 @@ public function validate($filePath) list($imageWidth, $imageHeight, $fileType) = getimagesize($filePath); if ($fileType) { if ($this->isImageType($fileType)) { - /** Check deprecated 'general/reprocess_images/active' for BC. If false then skip image reprocessing. */ - if (Mage::getStoreConfig('general/reprocess_images/active') !== null) { - $imageQuality = Mage::getStoreConfigFlag('general/reprocess_images/active') ? 100 : 0; + // Config 'general/reprocess_images/active' is deprecated, replacement is the following: + $imageQuality = Mage::getStoreConfig('admin/security/reprocess_image_quality'); + if ($imageQuality !== null) { + $imageQuality = (int) $imageQuality; } else { - $imageQuality = (int) Mage::getStoreConfig('admin/security/reprocess_image_quality'); + // Value not set in backend. For BC, if depcrecated config does not exist, default to 85. + $imageQuality = Mage::getStoreConfig('general/reprocess_images/active') === null + ? 85 + : (Mage::getStoreConfigFlag('general/reprocess_images/active') ? 85 : 0); } if ($imageQuality === 0) { return null; diff --git a/app/code/core/Mage/Core/etc/config.xml b/app/code/core/Mage/Core/etc/config.xml index 643f0e89a01..a0528aed8db 100644 --- a/app/code/core/Mage/Core/etc/config.xml +++ b/app/code/core/Mage/Core/etc/config.xml @@ -440,7 +440,6 @@ 1 10800 0 - 85 diff --git a/app/code/core/Mage/Core/etc/system.xml b/app/code/core/Mage/Core/etc/system.xml index 53411e945bc..eeb81099816 100644 --- a/app/code/core/Mage/Core/etc/system.xml +++ b/app/code/core/Mage/Core/etc/system.xml @@ -1282,8 +1282,8 @@ - Optimum value is 85, higher value will increase file size. Set to 0 to turn off images reprocessing, which may cause security risks. - required-entry validate-digits validate-digits-range digits-range-0-100 + Optimum value is 85 (default), higher value will increase file size. Set to 0 to turn off images reprocessing, which may cause security risks. + validate-digits validate-digits-range digits-range-0-100 180 1 1 diff --git a/app/locale/en_US/Mage_Core.csv b/app/locale/en_US/Mage_Core.csv index 924034de292..155c0df85d9 100644 --- a/app/locale/en_US/Mage_Core.csv +++ b/app/locale/en_US/Mage_Core.csv @@ -227,7 +227,7 @@ "New Website","New Website" "No","No" "Offloader header","Offloader header" -"Optimum value is 85, higher value will increase file size. Set to 0 to turn off images reprocessing, which may cause security risks.","Optimum value is 85, higher value will increase file size. Set to 0 to turn off images reprocessing, which may cause security risks." +"Optimum value is 85 (default), higher value will increase file size. Set to 0 to turn off images reprocessing, which may cause security risks.","Optimum value is 85 (default), higher value will increase file size. Set to 0 to turn off images reprocessing, which may cause security risks." "PHP SOAP extension is required.","PHP SOAP extension is required." "Package","Package" "Pagination","Pagination" From e795d54b3f10d50f063d863e008ccd211118fd45 Mon Sep 17 00:00:00 2001 From: kiat ng Date: Sat, 1 Oct 2022 20:45:28 +0800 Subject: [PATCH 4/5] Fixed bug on incorrect check if image quality was not set in backend. --- app/code/core/Mage/Core/Model/File/Validator/Image.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/code/core/Mage/Core/Model/File/Validator/Image.php b/app/code/core/Mage/Core/Model/File/Validator/Image.php index 215718c265b..4cfff748b89 100644 --- a/app/code/core/Mage/Core/Model/File/Validator/Image.php +++ b/app/code/core/Mage/Core/Model/File/Validator/Image.php @@ -88,7 +88,7 @@ public function validate($filePath) if ($this->isImageType($fileType)) { // Config 'general/reprocess_images/active' is deprecated, replacement is the following: $imageQuality = Mage::getStoreConfig('admin/security/reprocess_image_quality'); - if ($imageQuality !== null) { + if ($imageQuality != '') { $imageQuality = (int) $imageQuality; } else { // Value not set in backend. For BC, if depcrecated config does not exist, default to 85. From 25dd956c0d010b3e1ba0d81d486f14b216bd1651 Mon Sep 17 00:00:00 2001 From: kiat ng Date: Wed, 5 Oct 2022 09:24:18 +0800 Subject: [PATCH 5/5] Improved note in system.xml. --- app/code/core/Mage/Core/etc/system.xml | 2 +- app/locale/en_US/Mage_Core.csv | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/code/core/Mage/Core/etc/system.xml b/app/code/core/Mage/Core/etc/system.xml index eeb81099816..b54100f6e8e 100644 --- a/app/code/core/Mage/Core/etc/system.xml +++ b/app/code/core/Mage/Core/etc/system.xml @@ -1282,7 +1282,7 @@ - Optimum value is 85 (default), higher value will increase file size. Set to 0 to turn off images reprocessing, which may cause security risks. + The recommended value is 85, a higher value will increase the file size. You can set the value to 0 to disable image processing, but it may cause security risks. validate-digits validate-digits-range digits-range-0-100 180 1 diff --git a/app/locale/en_US/Mage_Core.csv b/app/locale/en_US/Mage_Core.csv index 155c0df85d9..4322d2f9f70 100644 --- a/app/locale/en_US/Mage_Core.csv +++ b/app/locale/en_US/Mage_Core.csv @@ -227,7 +227,7 @@ "New Website","New Website" "No","No" "Offloader header","Offloader header" -"Optimum value is 85 (default), higher value will increase file size. Set to 0 to turn off images reprocessing, which may cause security risks.","Optimum value is 85 (default), higher value will increase file size. Set to 0 to turn off images reprocessing, which may cause security risks." +"The recommended value is 85, a higher value will increase the file size. You can set the value to 0 to disable image processing, but it may cause security risks.","The recommended value is 85, a higher value will increase the file size. You can set the value to 0 to disable image processing, but it may cause security risks." "PHP SOAP extension is required.","PHP SOAP extension is required." "Package","Package" "Pagination","Pagination"