godot icon indicating copy to clipboard operation
godot copied to clipboard

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

Open nikitalita opened this issue 1 year ago • 4 comments

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

nikitalita avatar Oct 05 '24 22:10 nikitalita

CC @BlueCube3310

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

clayjohn avatar Oct 05 '24 22:10 clayjohn

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.

BlueCube3310 avatar Oct 06 '24 06:10 BlueCube3310

@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?

nikitalita avatar Oct 06 '24 09:10 nikitalita

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

kondoorsoft-trent avatar Oct 06 '24 14:10 kondoorsoft-trent

@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.

nikitalita avatar Oct 08 '24 19:10 nikitalita

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

fire avatar Oct 09 '24 05:10 fire