-
Notifications
You must be signed in to change notification settings - Fork 15.1k
[yaml2obj][MachO] Fix crash from integer underflow with invalid cmdsize #165924
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
base: main
Are you sure you want to change the base?
Conversation
yaml2obj would crash when processing Mach-O load commands with cmdsize smaller than the actual structure size e.g. LC_SEGMENT_64 with cmdsize=56 instead of 72. The crash occurred due to integer underflow when calculating padding: cmdsize - BytesWritten wraps to a large value when negative, causing a massive allocation attempt.
|
@llvm/pr-subscribers-objectyaml Author: Ryan Mansfield (rjmansfield) Changesyaml2obj would crash when processing Mach-O load commands with cmdsize smaller than the actual structure size e.g. LC_SEGMENT_64 with cmdsize=56 instead of 72. The crash occurred due to integer underflow when calculating padding: cmdsize - BytesWritten wraps to a large value when negative, causing a massive allocation attempt. Full diff: https://github.com/llvm/llvm-project/pull/165924.diff 2 Files Affected:
diff --git a/llvm/lib/ObjectYAML/MachOEmitter.cpp b/llvm/lib/ObjectYAML/MachOEmitter.cpp
index 35d442e8e3437..a3555b211ab53 100644
--- a/llvm/lib/ObjectYAML/MachOEmitter.cpp
+++ b/llvm/lib/ObjectYAML/MachOEmitter.cpp
@@ -285,7 +285,15 @@ void MachOWriter::writeLoadCommands(raw_ostream &OS) {
// Fill remaining bytes with 0. This will only get hit in partially
// specified test cases.
- auto BytesRemaining = LC.Data.load_command_data.cmdsize - BytesWritten;
+ // Prevent integer underflow if BytesWritten exceeds cmdsize.
+ if (BytesWritten > LC.Data.load_command_data.cmdsize) {
+ errs() << "warning: load command " << LC.Data.load_command_data.cmd
+ << " cmdsize too small (" << LC.Data.load_command_data.cmdsize
+ << " bytes) for actual size (" << BytesWritten << " bytes)\n";
+ }
+ auto BytesRemaining = (BytesWritten < LC.Data.load_command_data.cmdsize)
+ ? LC.Data.load_command_data.cmdsize - BytesWritten
+ : 0;
if (BytesRemaining > 0) {
ZeroFillBytes(OS, BytesRemaining);
}
diff --git a/llvm/test/ObjectYAML/MachO/segment-cmdsize-too-small.yaml b/llvm/test/ObjectYAML/MachO/segment-cmdsize-too-small.yaml
new file mode 100644
index 0000000000000..250a0631a5a9c
--- /dev/null
+++ b/llvm/test/ObjectYAML/MachO/segment-cmdsize-too-small.yaml
@@ -0,0 +1,33 @@
+## Test that yaml2obj handles cmdsize that is too small for LC_SEGMENT_64
+## without crashing (due to integer underflow).
+
+# RUN: yaml2obj %s -o %t 2>&1 | FileCheck %s --check-prefix=WARNING
+# RUN: not llvm-readobj --file-headers %t 2>&1 | FileCheck %s --check-prefix=MALFORMED
+
+# WARNING: warning: load command 25 cmdsize too small (56 bytes) for actual size (72 bytes)
+
+# MALFORMED: error: {{.*}}: truncated or malformed object (load command 0 LC_SEGMENT_64 cmdsize too small)
+
+--- !mach-o
+FileHeader:
+ magic: 0xFEEDFACF
+ cputype: 0x01000007
+ cpusubtype: 0x00000003
+ filetype: 0x00000001
+ ncmds: 1
+ sizeofcmds: 56
+ flags: 0x00002000
+ reserved: 0x00000000
+LoadCommands:
+ - cmd: LC_SEGMENT_64
+ cmdsize: 56 ## Should be 72 for LC_SEGMENT_64
+ segname: '__TEXT'
+ vmaddr: 0x1000
+ vmsize: 0x10
+ fileoff: 0
+ filesize: 0
+ maxprot: 7
+ initprot: 5
+ nsects: 0
+ flags: 0
+...
|
- Add getLoadCommandName() helper to map cmd values to names
- Change to indexed loop to show load command index for disambiguation
- Update warning format to match llvm-readobj style:
Known commands: "load command 0 LC_SEGMENT_64 cmdsize too small"
Unknown commands: "load command 0 (0xdeadbeef) cmdsize too small"
- Use WithColor::warning()
- Refactor warning message - Add testcase for unknown load command
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
- Do .str() around the full string concatenation - Reorganize test
jh7370
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, assuming pre-merge checks pass.
|
If this change is OK, can someone please merge it on my behalf? Thanks. |
yaml2obj would crash when processing Mach-O load commands with cmdsize smaller than the actual structure size e.g. LC_SEGMENT_64 with cmdsize=56 instead of 72. The crash occurred due to integer underflow when calculating padding: cmdsize - BytesWritten wraps to a large value when negative, causing a massive allocation attempt.