Skip to content

m68k: fix undefined abs() of INT32_MIN displacement in printer#2990

Closed
Samin061 wants to merge 1 commit into
capstone-engine:nextfrom
Samin061:m68k-abs-int-min
Closed

m68k: fix undefined abs() of INT32_MIN displacement in printer#2990
Samin061 wants to merge 1 commit into
capstone-engine:nextfrom
Samin061:m68k-abs-int-min

Conversation

@Samin061

@Samin061 Samin061 commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

Your checklist for this pull request

  • I've documented or updated the documentation of every API function and struct this PR changes.
  • I've added tests that prove my fix is effective or that my feature works (if possible)

Detailed description

printBaseDisp and printMemIndirect print a 32-bit displacement by calling abs() on op->mem.in_disp / op->mem.out_disp, but those fields are filled from read_imm_32 in the disassembler, so a crafted M68K memory-indirect operand can set them to INT32_MIN, where abs() negates a value with no representable positive counterpart and the behavior is undefined. The magnitude now goes through a small unsigned-negation helper so it stays well-defined at the three 32-bit sites; the sign is already printed separately, so the output is unchanged. The int16_t disp sites cannot reach INT32_MIN and are left alone.

Test plan

Building with -fsanitize=undefined and running test_poc aborts at M68KInstPrinter.c on cs_disasm of the new test_ub_abs_m68k_disp input before the change and exits cleanly after. That case disassembles a full-format memory-indirect move.w whose outer displacement is 0x80000000.

Closing issues

@github-actions github-actions Bot added the M68K Arch label Jul 8, 2026
@Rot127

Rot127 commented Jul 8, 2026

Copy link
Copy Markdown
Collaborator

There is literally an util function for it.
Closing it for the same reason as #2989 (comment)
As said, feel free to fix the issue again after properly evaluating the code change.

@Rot127 Rot127 closed this Jul 8, 2026
@Samin061

Samin061 commented Jul 9, 2026

Copy link
Copy Markdown
Contributor Author

Took another look at this. printInt32HexOffset/printInt16HexOffset do handle the same UB, but they emit the offset as +0x.../-0x..., which changes the M68K $ syntax and the existing m68k.yaml detail case (e.g. ([$7fffffff, a0], d0.w, $12345678) would turn into ([+0x7fffffff, ...], ..., +0x12345678)). So they aren't a drop-in here.

To keep the disassembly output byte-for-byte identical I dropped the local helper and used the same well-defined unsigned negation those utils rely on internally, inline at the three 32-bit sites. I also added a cstest case in tests/details/m68k.yaml asserting the INT32_MIN outer displacement prints as -$80000000, and checked it with cstool plus a clean UBSan run.

I pushed the update to the branch, but GitHub won't let me reopen since it was force-pushed. If you'd rather standardize on the 0x offset form I'm happy to switch to the util and update the affected M68K tests instead. Let me know how you'd like to proceed (reopen on your side, or a fresh PR).

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

Labels

M68K Arch

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants