Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
nikitalita opened this issue Oct 5, 2024 · 6 comments · Fixed by #97873
Closed

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

nikitalita opened this issue Oct 5, 2024 · 6 comments · Fixed by #97873

Comments

@nikitalita
Copy link
Contributor

Tested versions

godot master @ 5ccbf6e

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
Copy link
Member

clayjohn commented Oct 5, 2024

CC @BlueCube3310

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

@BlueCube3310
Copy link
Contributor

BlueCube3310 commented Oct 6, 2024

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
Copy link
Contributor Author

@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
Copy link

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

@nikitalita
Copy link
Contributor Author

@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
Copy link
Member

fire commented Oct 9, 2024

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants