Problem with certain PNG images

[klerebladh]

Hi,

We've found an issue with certain .png images that causes a crash in our system when processed. There seem to be some heap corruption going on but I leave that to you to deduce. The crash happened sort of intermittently in our system but I managed to reproduce it in a console app when explicitly triggering a garbage collection (see code below). Attaching the "bad" image as well. This issue seems to have been introduced with v0.11 as I don't see the problem in v0.10.3.

`using var originalStream = File.OpenRead("bad-file.png"); using var thumbnailStream = new MemoryStream(); var settings = new ProcessImageSettings { Width = 400, Height = 400, ResizeMode = CropScaleMode.Max, JpegQuality = 76, HybridMode = HybridScaleMode.Turbo, SaveFormat = FileFormat.Png };

MagicImageProcessor.ProcessImage(originalStream, thumbnailStream, settings); GC.Collect();`

[saucecontrol]

Thanks for the report! I have been able to reproduce the crash and am working on a fix.

[saucecontrol]

Well, I've tracked the crash down to a buffer overrun, but it's coming from a Windows component (WIC/WCS). I can't reproduce it on Win11, but it does reproduce reliably on Win10. Can you tell me what OS versions you've been using when you encountered the crashes?

What's unusual about your sample PNG is that it has an embedded color profile defining an RGB LUT conversion (Apple Wide Sharing Profile), which is not normally seen in combination with RGBA format images, although I see no reason that shouldn't be valid. The easy fix might be to tell Windows not to attempt the conversion, but I'd like to try to narrow down the cause a bit more before resorting to that.

[klerebladh]

Happy to hear that you have tracked down the issue! I have reproduced the crash on my dev machine running Win 11 22H2 OS-version 22621.1992 but have also seen it on a Win 10 OS-version 10.0.19041.1.

[saucecontrol]

Thanks for the extra info. I found the root cause, which was that the WIC color transformer will write more pixels than requested in some situations. It does, however, respect the max buffer size it's given, so I've made a change to restrict the reported buffer length to only what the transform is allowed to touch.

That cleared up the issue you reported, and I've audited and fixed a few other places that could have been affected in theory. Fix is in https://github.com/saucecontrol/PhotoSauce/commit/5c91cc6e9215a8b1976ce8bd63a15993f0db273c and a new set of packages are in the CI NuGet feed

[andreas-eriksson]

Will there be a public release for this as well?

[saucecontrol]

Yep, I'm still planning on a release snap this week if I can get a couple of days to focus on it. Otherwise, soon™

[svenclaesson]

This file works but now but for unknown reason MagicScaler tries to load lcms2 when using WIC (not when using libpng) but this exception is handled.

Exception thrown: 'System.DllNotFoundException' in System.Private.CoreLib.dll Unable to load DLL 'lcms2' or one of its dependencies: The specified module could not be found. (0x8007007E)

This could potentially lead to DLL preloading attack

[saucecontrol]

lcms2 is used opportunistically on Windows, for a few reasons

1) It has capabilities WIC's color management doesn't, such as conversion of CMYK with alpha channel and 16bpc CMYK. 2) Its conversion is higher quality than the Windows Color System used by WIC. 3) It allows testing of all native plugin paths on Windows.

lcms2 is likely a temporary dependency, as I plan to build this functionality into the managed library at some point.

As to DLL preloading attacks, the call to lcms2 is done through DllImport, so it is no more a vulnerability than any other P/Invoke. If threat modeling for your app includes the possibility an attacker would have access to your system and the ability to place a malicious .dll in the default search paths or alter the search paths (while somehow not also having access to modify the app or its local dependencies), you should mitigate that in your app. It's not something I can do from the library without potential negative impact to the apps using it.