Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
257 changes: 257 additions & 0 deletions Model/StrictImagePayloadValidator.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,257 @@
<?php

declare(strict_types=1);

namespace MarkShust\PolyshellPatch\Model;

use Magento\Framework\Api\Data\ImageContentInterface;
use Magento\Framework\Exception\LocalizedException;
use Magento\Framework\Phrase;

/**
* OWASP File Upload Cheat Sheet–aligned checks for image payloads:
* verify file signatures (magic bytes), confirm decoded content type matches the declared MIME,
* cap raw size and pixel dimensions before decode, then re-encode with GD (required) so only
* raster data remains (strips appended polyglot bytes, EXIF, etc.).
*
* Animated GIFs: GD outputs the first frame only — document for merchants if needed.
*
* @see https://cheatsheetseries.owasp.org/cheatsheets/File_Upload_Cheat_Sheet.html
*/
class StrictImagePayloadValidator
{
/**
* User-facing invalid-image text (keep in sync with {@see self::getInvalidImagePhrase()}).
*/
public const FAILURE_MESSAGE = 'The image content is invalid. Verify the content and try again.';

/** Maximum decoded payload size (bytes) before image APIs run — 5 MiB. */
public const MAX_DECODED_BYTES = 5242880;

/**
* Reject Base64 input longer than this before base64_decode (no valid payload can exceed MAX_DECODED_BYTES).
* Bound: largest L with ⌊L·3/4⌋ ≤ MAX_DECODED_BYTES.
*/
public const MAX_BASE64_INPUT_CHARS = 6990507;

/** Maximum width/height from getimagesize (IHDR etc.) before imagecreatefromstring — mitigates decompression-bomb headers. */
public const MAX_IMAGE_WIDTH = 10000;

public const MAX_IMAGE_HEIGHT = 10000;

private const FORMAT_JPEG = 'jpeg';
private const FORMAT_PNG = 'png';
private const FORMAT_GIF = 'gif';

/**
* Translated phrase for invalid image payload.
*
* @return Phrase
*/
public function getInvalidImagePhrase(): Phrase
{
return __('The image content is invalid. Verify the content and try again.');
}

/**
* Validate and normalize image payload (magic bytes, MIME, caps, GD re-encode).
*
* @param string $binary Strict base64-decoded bytes
* @param string $declaredMime e.g. image/jpeg
* @param ImageContentInterface $imageContent
* @return void
* @throws LocalizedException
*/
public function assertValidAndNormalize(
string $binary,
string $declaredMime,
ImageContentInterface $imageContent
): void {
if ($binary === '') {
$this->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;
}
}
9 changes: 4 additions & 5 deletions Plugin/ImageContentValidatorExtension.php
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -41,7 +40,7 @@ public function __construct(IoFile $ioFile)
* @param bool $result
* @param ImageContentInterface $imageContent
* @return bool
* @throws InputException
* @throws LocalizedException
*
* @SuppressWarnings(PHPMD.UnusedFormalParameter)
*/
Expand All @@ -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)
);
}

Expand Down
74 changes: 74 additions & 0 deletions Plugin/ImageProcessorStrictBinaryValidation.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
<?php

declare(strict_types=1);

namespace MarkShust\PolyshellPatch\Plugin;

use Magento\Framework\Api\Data\ImageContentInterface;
use Magento\Framework\Api\ImageProcessor;
use Magento\Framework\Exception\LocalizedException;
use MarkShust\PolyshellPatch\Model\StrictImagePayloadValidator;

/**
* Runs strict binary validation and GD re-encode before media write.
*
* Size limits: {@see StrictImagePayloadValidator::MAX_BASE64_INPUT_CHARS} and
* {@see StrictImagePayloadValidator::MAX_DECODED_BYTES} (reject oversized input before full decode).
*/
class ImageProcessorStrictBinaryValidation
{
/**
* @var StrictImagePayloadValidator
*/
private StrictImagePayloadValidator $strictImagePayloadValidator;

/**
* @param StrictImagePayloadValidator $strictImagePayloadValidator
*/
public function __construct(StrictImagePayloadValidator $strictImagePayloadValidator)
{
$this->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);
}
}
3 changes: 2 additions & 1 deletion composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
6 changes: 5 additions & 1 deletion etc/di.xml
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,12 @@
<config xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:noNamespaceSchemaLocation="urn:magento:framework:ObjectManager/etc/config.xsd">
<type name="Magento\Framework\Api\ImageProcessor">
<plugin name="markshust_polyshell_strict_binary_validation"
type="MarkShust\PolyshellPatch\Plugin\ImageProcessorStrictBinaryValidation"
sortOrder="5"/>
<plugin name="markshust_polyshell_restrict_upload_extensions"
type="MarkShust\PolyshellPatch\Plugin\ImageProcessorRestrictExtensions"/>
type="MarkShust\PolyshellPatch\Plugin\ImageProcessorRestrictExtensions"
sortOrder="10"/>
</type>
<type name="Magento\Framework\Api\ImageContentValidator">
<plugin name="markshust_polyshell_validate_file_extension"
Expand Down