Skip to content

Fix stack buffer overflow in ADMesh stl_read (unbounded solid name) - #233

Open
MAVProxyUser wants to merge 1 commit into
QIDITECH:mainfrom
MAVProxyUser:fix-admesh-solid-name-and-mw-overflow
Open

Fix stack buffer overflow in ADMesh stl_read (unbounded solid name)#233
MAVProxyUser wants to merge 1 commit into
QIDITECH:mainfrom
MAVProxyUser:fix-admesh-solid-name-and-mw-overflow

Conversation

@MAVProxyUser

Copy link
Copy Markdown

Fix stack buffer overflow in the ADMesh ASCII-STL loader (stl_read)

Summary

stl_read() in src/admesh/stlinit.cpp copies the ASCII-STL solid name into a fixed 256-byte stack buffer (solid_content[256]) with an fscanf scanset that has no field width, so a solid name longer than 255 bytes overruns the buffer and the adjacent saved stack state, including the saved return address. Reachable simply by opening a .stl file.

Why QIDI is affected

QIDI forked BambuStudio directly, so it inherited the ADMesh loader (src/admesh/stlinit.cpp) and with it the unbounded solid-name fscanf. The defect is present at HEAD on the default branch.

Impact

  • Memory corruption on file openinstruction-pointer control (the saved return address is overwritten with attacker bytes).
  • No Pointer Authentication on the shipped macOS build: the distributed arm64 slice is plain arm64 (not arm64e), so the saved return address is a raw, directly-usable code pointer.
  • Trivially reachable: no user interaction beyond opening a .stl a user received from a third party (marketplace, shared project, print farm).

Vulnerable code

char solid_content[256];
int res_solid = fscanf(fp, " solid %[^\n]", solid_content);   // no field width

Proof — return address in control

Generate a cyclic (De Bruijn) solid name, open the file, and read the crash report's own backtrace; the saved return-address slot (frame #1) contains the pattern bytes, decoding to solid-name offset 368. A plain 20000×'A' name puts 0x414141414141 in the same slot. (Full reproducer and analysis: see the companion BambuStudio and OrcaSlicer reports linked below.)

Suggested fix (applied in this PR)

Bound every conversion with an explicit field width matching the destination:

-            int res_solid = fscanf(fp, " solid %[^\n]", solid_content);
+            int res_solid = fscanf(fp, " solid %255[^\n]", solid_content);

%255[^\n] keeps the existing behavior (the name is still read) while making overflow impossible. As general hardening, every %s/%[ scanset targeting a fixed buffer in this file should carry an explicit maximum field width.

Companion reports

🤖 Generated with Claude Code

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant