assimp / assimp/assimp

Integer Overflow in `MDLImporter::CreateTextureARGB8_3DGS_MDL3()`

Open
#6,226 1 comment 0 reactions 0 assignees View on GitHub
Bug Fuzzer MDL Sanitizer
Dominant language
C++
Stars
13.2k
Forks
3.2k
Avg merge
2d 8h
Merged PRs (30d)
21

Description

**Describe the bug**
Integer overflow can happen in function `MDLImporter::CreateTextureARGB8_3DGS_MDL3()` if two high numbers are provided in the multiplication in line [127](https://github.com/assimp/assimp/blob/8621d5243070eb85ad7411634b5463731fdac7ed/code/AssetLib/MDL/MDLMaterialLoader.cpp#L127).

This value is then checked for validity through `VALIDATE_FILE_SIZE()` which will in the end call `SizeCheck()` is `IsPosValid()`. The problem here is that, with the overflow occuring, the resulting value that is checked in these last two functions can be valid, as part of it will likely be truncated because of the overflow, resulting in an undetermined computation.

I can see that, a few lines after this error occurs, you are already checking for another integer overflow (this last check avoids using the result of an overflow to access memory), you can fix this error with the same approach by detecting the overflow before it happens and raising an error, with something like:
```
if(pcHeader->skinwidth != 0 && pcHeader->skinheight > UINT_MAX/pcHeader->skinwidth) {
throw DeadlyImportError("Invalid MDL file. [...]");
}
```
before performing the multiplication.

**Reproduce**
You can reproduce the error by using the file in the [attached archive](https://github.com/user-attachments/files/20613303/1.zip). We tested your fuzzing harness compiling with LLVM 20 and AFL++ 4.32, on Ubuntu 20.04. To detect this issue, UBSan is needed.

Contributor guide

Open the contributing guide

Research direction

Start in code/AssetLib/MDL/MDLMaterialLoader.cpp at MDLImporter::CreateTextureARGB8_3DGS_MDL3(), especially line 127 and the nearby overflow check. Reproduce with the attached archive using the fuzzing harness and UBSan, then verify that the high-value multiplication is rejected before its result reaches validation or memory access.

Written by the indexing model from the issue text.

Assessment

Tech stack
cpp
Domain
security
Issue type
Bug
Difficulty
2/5
Estimated time
1-3 hours
Activity status
Stale
Clarity
Clearly specified
Newbie friendliness
55/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.