godotengine / godot

Godot Engine – Multi-platform 2D and 3D game engine
https://godotengine.org
MIT License
90.09k stars 21.14k forks source link

Heap buffer overflow when attempting to decompress a 1x1 DXT1 compressed image #97862

Closed nikitalita closed 1 day ago

nikitalita commented 6 days ago

Tested versions

godot master @ 5ccbf6e4c794a4e47456edd9434b75fcd6096a2f

System information

macOS 14.5.0, M2 Pro, Vulkan

Issue description

Attempting to call decompress() on a 1x1 DXT texture image (less than the 4x4 required) results in misaligned writes, which often lead to a heap buffer overflow. Sometimes you get lucky with your memory layout, but this is pretty reproducible for me.

The culprit:

image

Happens right here, in bcdec.h:

image

Attached below is an ASAN log with the full stacktrace: asan.log

Steps to reproduce

Open up the reproduction project in the editor; it will likely just crash immediately, though you may get lucky/unlucky depending on your memory layout.

Minimal reproduction project (MRP)

testdxtdecompress.zip

clayjohn commented 6 days ago

CC @BlueCube3310

I guess we need to do an early out if the texture dimensions are smaller than the block size

BlueCube3310 commented 6 days ago

1x1 DXT images shouldn't even be possible, I think it's an issue with how the compressed textures with non-multiple of 4 images retain their original dimensions, instead of their padded ones.

In the decompressor we could round the resolution of the compressed images to the nearest upper multiple of 4, since that should fix most crashes.

nikitalita commented 6 days ago

@kondoorsoft-trent would you be willing to upload the original SmallShip01.glb and SmallShip02.glb so that we can figure out how it got imported like that?

kondoorsoft-trent commented 5 days ago

Sure! Here are both, packed into a ZIP SmallShips.zip

nikitalita commented 3 days ago

@fire This appears to be an issue with the glb importer as well. The embedded texture is 1x1; If we're importing these as DXT textures, they should be padded to 4x4.

fire commented 3 days ago

@lyuma argued a long time ago that 1x1 dxt are allowed via a smearing algorithm but I don’t have the log anymore