Malicious hand-crafted PNG can be used to trigger DOS attack

[kroymann]

Prerequisites

• I have written a descriptive issue title
• I have verified that I am running the latest version of ImageSharp
• I have verified if the problem exist in both DEBUG and RELEASE mode
• I have searched open and closed issues to ensure it has not already been reported

ImageSharp version

3.1.4

Other ImageSharp packages and versions

ImageSharp.Web 3.1.2

Environment (Operating system, version and so on)

Azure App Service (Windows)

.NET Framework version

7.0

Description

A malicious user uploaded the attached image into our system. The PNG header specifies that the image is 1 pixel wide by 500M pixels tall. Attempting to load that image results in massive memory allocations and CPU load, and if the load actually succeeds without bringing down the server, subsequent operations such as resizing result in an InvalidMemoryOperationException as it attempts to allocate multiple GB of memory:

SixLabors.ImageSharp.Memory.InvalidMemoryOperationException: Attempted to allocate a buffer of length=8000000004 that exceeded the limit 1073741824.
   at SixLabors.ImageSharp.Memory.InvalidMemoryOperationException.ThrowAllocationOverLimitException
   at SixLabors.ImageSharp.Memory.UniformUnmanagedMemoryPoolMemoryAllocator.Allocate
   at SixLabors.ImageSharp.Memory.MemoryAllocatorExtensions.Allocate2D
   at SixLabors.ImageSharp.Processing.Processors.Transforms.ResizeKernelMap..ctor
   at SixLabors.ImageSharp.Processing.Processors.Transforms.ResizeKernelMap.Calculate
   at SixLabors.ImageSharp.Processing.Processors.Transforms.ResizeProcessor`1.ApplyTransform
   at SixLabors.ImageSharp.Processing.Processors.Transforms.ResizeProcessor`1.BeforeImageApply
   ...

Ideally loading this image (and others like it) would fail fast and treat it as a malformed image. Another possible option could be to have a configuration setting to specify the maximum allowable image dimensions that could then then be checked and validated during image loading so that the load could abort when faced with such a ridiculously large image.

Steps to Reproduce

Simply call Image.Load() referencing the attached PNG file.

Images

[JimBobSquarePants]

Are you sure there is the allocation? That error is explicitly thrown because we are preventing the allocation as it exceeds the configurable threshold. You can configure your own limits.

https://docs.sixlabors.com/articles/imagesharp/security.html

CC @antonfirsov

[kroymann]

There's a large initial allocation when creating the ImageFrame. It allocates somewhere in the ballpark of 1.6GB. The exception that I quoted above came from our logs when a server instance had successfully managed to load this image into memory, and was then trying to resize it to a 512-pixel width (per a client request).

[kroymann]

You can configure your own limits.
https://docs.sixlabors.com/articles/imagesharp/security.html

Ah good point. So rather than try to limit the image dimensions, I could instead pick a reasonable memory allocation limit, which would have stopped the image load before it allocated ~1.6GB and then spun for a really long time reading 500M scan lines of data.

[kroymann]

With that said, even if I set that limit considerably lower (say low enough to just barely support a 4K UHD image - 3840x2160), this specific attack would still be dangerous. The attacker could change their hand-crafted PNG to specify that the image is 8M pixels tall rather than 500M as the current attack did, which would then fit inside my more aggressive memory allocation limit (since it's only a single pixel wide). And then trying to load the image would again result in the CPU spinning for a really long time as it reads in 8M scan lines. It still feels like having an extra configuration option that lets you specify a limit on image dimensions would be a good safe-guard here.

[JimBobSquarePants]

Why not read the ImageInfo first? You can do that without allocating and reject anything too large.

The reason we use a memory limit instead of a dimensional limit is because images of pixel formats use different memory amounts. A single property is far easier to manage.

[kroymann]

Why not read the ImageInfo first? You can do that without allocating and reject anything too large.

We're using the ImageSharp.Web library to handle loading and manipulating images. The extensibility points that we could potentially leverage here are the IImageProvider/IImageResolver interfaces, and the OnXxxx delegates defined in the ImageSharpMiddlewareOptions. But unless I'm missing something obvious, I don't see a good way to hook into the middleware pipeline and perform an ImageInfo check prior to the call to FormattedImage.LoadAsync().

[kroymann]

Actually, looking at that code a bit more, I think I could squeeze an ImageInfo check into the IImageResolver.OpenReadAsync() method as I will have access to the stream at that point. The one downside to that approach is that if the stream being returned by the resolver is not natively seekable, then I'll need to duplicate the WithSeekableStream() behavior that is currently internal to ImageSharp so I can reset the position after doing the ImageInfo check.

[JimBobSquarePants]

If you are using Web how are the users uploading the image to make the request? You should control input there.

Are you also using HMAC to protect the actual requests?

[kroymann]

If you are using Web how are the users uploading the image to make the request? You should control input there.

Raw source images are uploaded into our system through a separate service, and then requests are sent from clients to the ImageSharp-based service to obtain permutations of those images (eg: resized to various smaller dimensions, cropped to a square aspect ratio, etc). We have numerous layers of defense to try and prevent malicious users from injecting bad content into our system, but it is a constant game of cat-and-mouse and every security barrier we erect is eventually surmounted by hackers. We fundamentally do not have a way to 100% trust the clients and so I'd like to try and have the ImageSharp-based service be as robust as possible in the face of potentially malicious content making it past our filters.

Are you also using HMAC to protect the actual requests?

No, although we already have defenses in place to sanitize and reject image transformation query params that are outside the bounds of what we expect and can handle (as our servers are continuously being hit with automated fuzz attacks as well as hackers trying to manually manipulate the URLs to see what they can accomplish).

[JimBobSquarePants]

Raw source images are uploaded into our system through a separate service

This is where I would be inspecting those inputs and rejecting invalid ones.

[kroymann]

Raw source images are uploaded into our system through a separate service

This is where I would be inspecting those inputs and rejecting invalid ones.

Yep that's a fair point. We weren't currently using ImageSharp to parse/inspect content as it was being uploaded into our system (it was only being used on the consumption side), but I think we've reached a point where that is the next necessary defense.

Thank you for your feedback (and suggestion to customize the AllocationLimitMegabytes). I think we can probably go ahead and close this issue if you'd like. :)

[antonfirsov]

CPU spinning for a really long time as it reads in 8M scan lines.

Ah yeah, when pushing for AllocationLimitMegabytes instead of dimension limits I didn't consider that reading a scanline has some overhead compared to reading individual pixels within the same scanline, but does this really change the big picture?

The nature of PNG is that small, low-entropy files can expand to a much (~30000x) bigger area in memory compared to the input size while also consuming noticeble amount of CPU% to fill that memory with content. This means that even if you run Identify somewhere & set your dimension limits to reasonable values, it would be still possible to do cheap attacks with valid images that pass trough your filter by sending them to your service massively in parallel. Do you have any measures to mitigate this?

We have numerous layers of defense to try and prevent malicious users from injecting bad content into our system

I'm curious about the list of mechanisms you end up applying to secure your ImageSharp(.Web) usage. https://docs.sixlabors.com/articles/imagesharp/security.html feels like a chunk in it's current form, it would be nice to extend it with further (and more specific) recommendations. Keep us in the loop if you have the time!

[kroymann]

I'm curious about the list of mechanisms you end up applying to secure your ImageSharp(.Web) usage. https://docs.sixlabors.com/articles/imagesharp/security.html feels like a chunk in it's current form, it would be nice to extend it with further (and more specific) recommendations. Keep us in the loop if you have the time!

On the consumption side, the only thing we've done is add a simple sanitization pass on the query parameters in order to reject requests that don't tightly conform to expectations. Our server only supports a limited set of transformations with the sole purpose being to leverage client-side knowledge of how the requested image will be displayed in order to optimize bandwidth (ie: resizing to various smaller resolutions, cropping to a square aspect ratio, etc). The external clients come in multiple forms: a game that runs on PCs/game-consoles/phones/VR-headsets/etc, public facing websites, and internal admin tools/websites. All traffic routes through a CDN, with the ImageSharp service acting as the origin behind that CDN.

The upload side has a more defenses, requiring user authentication from a valid game client (hackers use tools to extract a transient authentication token from RAM on the game client and then impersonate the game client) and a custom HTTP request signing algorithm (which hackers reverse engineered from game client binaries). We've also been building up a library of content validation filters that inspect the contents of the uploaded files before allowing them into the system. Per this discussion, I will likely be beefing up those validation filters to do more parsing & validation of images.

[JimBobSquarePants]

Ah yeah, when pushing for AllocationLimitMegabytes instead of dimension limits I didn't consider that reading a scanline has some overhead compared to reading individual pixels within the same scanline, but does this really change the big picture?

Within the PNG decoder. we reuse the same scanline buffer when assigning pixel values and also limit temp buffer allocations used when inflating the compressed IDAT chunk. You were absolutely correct to push a memory limit over dimensional.

I'm not convinced we have anything to action here. We provide configurable limits and the means to inspect image data before decoding. That's as much if not more than any other library.