diff --git a/Model/StrictImagePayloadValidator.php b/Model/StrictImagePayloadValidator.php new file mode 100644 index 0000000..981ae58 --- /dev/null +++ b/Model/StrictImagePayloadValidator.php @@ -0,0 +1,257 @@ +fail(); + } + + if (strlen($binary) > self::MAX_DECODED_BYTES) { + $this->fail(); + } + + if (!extension_loaded('gd') || !function_exists('imagecreatefromstring')) { + $this->fail(); + } + + $this->assertMagicBytesAndParserMimeMatchDeclared($binary, $declaredMime); + $this->reencodeWithGd($binary, $declaredMime, $imageContent); + } + + /** + * Throw standardized invalid-image exception. + * + * @return never + * @throws LocalizedException + */ + private function fail(): void + { + throw new LocalizedException($this->getInvalidImagePhrase()); + } + + /** + * Verify magic bytes and parser MIME match declared type. + * + * List allowed extensions → verify magic bytes; verify file content matches extension (parser MIME vs declared). + * Parallels OWASP: allowlist + verify file signature + verify content matches type. + * + * @param string $binary + * @param string $declaredMime + * @return void + */ + private function assertMagicBytesAndParserMimeMatchDeclared(string $binary, string $declaredMime): void + { + $expectedFormat = $this->declaredMimeToFormat($declaredMime); + if ($expectedFormat === null) { + $this->fail(); + } + + $magicFormat = $this->detectFormatFromMagicBytes($binary); + if ($magicFormat === null || $magicFormat !== $expectedFormat) { + $this->fail(); + } + + $props = getimagesizefromstring($binary); + if ($props === false || empty($props['mime']) || empty($props[0]) || empty($props[1])) { + $this->fail(); + } + + if ($this->declaredMimeToFormat($props['mime']) !== $expectedFormat) { + $this->fail(); + } + + if ($props[0] > self::MAX_IMAGE_WIDTH || $props[1] > self::MAX_IMAGE_HEIGHT) { + $this->fail(); + } + } + + /** + * Map MIME type to internal format key. + * + * @param string $mime + * @return string|null + */ + private function declaredMimeToFormat(string $mime): ?string + { + $mime = strtolower(trim($mime)); + switch ($mime) { + case 'image/jpeg': + case 'image/jpg': + return self::FORMAT_JPEG; + case 'image/png': + return self::FORMAT_PNG; + case 'image/gif': + return self::FORMAT_GIF; + default: + return null; + } + } + + /** + * Detect image format from magic bytes. + * + * @param string $binary + * @return string|null + */ + private function detectFormatFromMagicBytes(string $binary): ?string + { + if (strlen($binary) < 8) { + return null; + } + + if (strncmp($binary, "\xFF\xD8\xFF", 3) === 0) { + return self::FORMAT_JPEG; + } + + if (strncmp($binary, "\x89PNG\r\n\x1a\n", 8) === 0) { + return self::FORMAT_PNG; + } + + if (strlen($binary) >= 6 + && (strncmp($binary, 'GIF87a', 6) === 0 || strncmp($binary, 'GIF89a', 6) === 0) + ) { + return self::FORMAT_GIF; + } + + return null; + } + + /** + * Re-encode with GD and replace base64 on the DTO. + * + * @param string $binary + * @param string $declaredMime + * @param ImageContentInterface $imageContent + * @return void + */ + private function reencodeWithGd( + string $binary, + string $declaredMime, + ImageContentInterface $imageContent + ): void { + // phpcs:disable Magento2.Functions.DiscouragedFunction -- GD decode/destroy required for re-encode pipeline + $image = imagecreatefromstring($binary); + if ($image === false) { + $this->fail(); + } + + try { + $output = $this->encodeGdResourceToDeclaredMime($image, $declaredMime); + } finally { + imagedestroy($image); + } + // phpcs:enable Magento2.Functions.DiscouragedFunction + + $imageContent->setBase64EncodedData(base64_encode($output)); + } + + /** + * Encode GD resource to output bytes for the declared MIME type. + * + * @param \GdImage|resource $image + * @param string $declaredMime + * @return string + */ + private function encodeGdResourceToDeclaredMime($image, string $declaredMime): string + { + // phpcs:disable Magento2.Functions.DiscouragedFunction -- GD output capture requires output buffering and image*() + ob_start(); + try { + switch ($declaredMime) { + case 'image/jpeg': + case 'image/jpg': + if (!imagejpeg($image, null, 92)) { + $this->fail(); + } + break; + case 'image/png': + if (!imagepng($image, null, 6)) { + $this->fail(); + } + break; + case 'image/gif': + if (!imagegif($image)) { + $this->fail(); + } + break; + default: + $this->fail(); + } + $buffer = ob_get_contents(); + } finally { + ob_end_clean(); + } + // phpcs:enable Magento2.Functions.DiscouragedFunction + + if ($buffer === false || $buffer === '') { + $this->fail(); + } + + return $buffer; + } +} diff --git a/Plugin/ImageContentValidatorExtension.php b/Plugin/ImageContentValidatorExtension.php index a7dd627..2f1614c 100644 --- a/Plugin/ImageContentValidatorExtension.php +++ b/Plugin/ImageContentValidatorExtension.php @@ -6,9 +6,8 @@ use Magento\Framework\Api\Data\ImageContentInterface; use Magento\Framework\Api\ImageContentValidator; -use Magento\Framework\Exception\InputException; +use Magento\Framework\Exception\LocalizedException; use Magento\Framework\Filesystem\Io\File as IoFile; -use Magento\Framework\Phrase; /** * Validate that the uploaded filename has a safe image extension. @@ -41,7 +40,7 @@ public function __construct(IoFile $ioFile) * @param bool $result * @param ImageContentInterface $imageContent * @return bool - * @throws InputException + * @throws LocalizedException * * @SuppressWarnings(PHPMD.UnusedFormalParameter) */ @@ -55,8 +54,8 @@ public function afterIsValid( $extension = strtolower($pathInfo['extension'] ?? ''); if ($extension && !in_array($extension, self::ALLOWED_EXTENSIONS, true)) { - throw new InputException( - new Phrase('The image file extension "%1" is not allowed.', [$extension]) + throw new LocalizedException( + __('The image file extension "%1" is not allowed.', $extension) ); } diff --git a/Plugin/ImageProcessorStrictBinaryValidation.php b/Plugin/ImageProcessorStrictBinaryValidation.php new file mode 100644 index 0000000..8d6d927 --- /dev/null +++ b/Plugin/ImageProcessorStrictBinaryValidation.php @@ -0,0 +1,74 @@ +strictImagePayloadValidator = $strictImagePayloadValidator; + } + + /** + * Validate payload before ImageProcessor writes to media. + * + * @param ImageProcessor $subject + * @param callable $proceed + * @param string $entityType + * @param ImageContentInterface $imageContent + * @return string + * @throws LocalizedException + * + * @SuppressWarnings(PHPMD.UnusedFormalParameter) + */ + public function aroundProcessImageContent( + ImageProcessor $subject, + callable $proceed, + $entityType, + ImageContentInterface $imageContent + ) { + $encoded = $imageContent->getBase64EncodedData(); + if ($encoded === null || $encoded === '') { + throw new LocalizedException(__('The image content is invalid. Verify the content and try again.')); + } + + if (strlen($encoded) > StrictImagePayloadValidator::MAX_BASE64_INPUT_CHARS) { + throw new LocalizedException(__('The image content is invalid. Verify the content and try again.')); + } + + // phpcs:ignore Magento2.Functions.DiscouragedFunction -- strict mode; invalid base64 returns false + $binary = base64_decode($encoded, true); + if ($binary === false || $binary === '') { + throw new LocalizedException(__('The image content is invalid. Verify the content and try again.')); + } + + $this->strictImagePayloadValidator->assertValidAndNormalize( + $binary, + (string) $imageContent->getType(), + $imageContent + ); + + return $proceed($entityType, $imageContent); + } +} diff --git a/composer.json b/composer.json index 1dc0e06..687ed5b 100644 --- a/composer.json +++ b/composer.json @@ -3,7 +3,8 @@ "description": "The PolyshellPatch module mitigates the PolyShell vulnerability (APSB25-94) — an unrestricted file upload in the Magento REST API that allows attackers to upload executable files via cart item custom option file uploads.", "require": { "php": "^8.1", - "magento/framework": "^103" + "magento/framework": "^103", + "ext-gd": "*" }, "require-dev": { "roave/security-advisories": "dev-latest" diff --git a/etc/di.xml b/etc/di.xml index 604c99d..fd3209c 100644 --- a/etc/di.xml +++ b/etc/di.xml @@ -2,8 +2,12 @@ + + type="MarkShust\PolyshellPatch\Plugin\ImageProcessorRestrictExtensions" + sortOrder="10"/>