[llvm] [MachO] Loosen boundary check for reading export trie nodes (PR #96705)
Cyndy Ishida via llvm-commits
llvm-commits at lists.llvm.org
Tue Jun 25 19:08:59 PDT 2024
https://github.com/cyndyishida updated https://github.com/llvm/llvm-project/pull/96705
>From 24af0cb406f8e87ea1027e9a464564c9b3ef1d0c Mon Sep 17 00:00:00 2001
From: Cyndy Ishida <cyndy_ishida at apple.com>
Date: Tue, 25 Jun 2024 07:55:13 -0700
Subject: [PATCH 1/2] [MachO] Loosen boundary check for reading export trie
nodes
The design of the export trie in macho's is that each node has a variable length payload. When reading nodes, it should be an error if reading the uleb128 puts you beyond the stated node size but not when the stated size goes beyond the known part that was read.
resolves: rdar://130310832
---
llvm/lib/Object/MachOObjectFile.cpp | 2 +-
.../MachO/Inputs/macho-inconsistent-export | Bin 8752 -> 0 bytes
.../tools/llvm-objdump/MachO/bad-trie.test | 3 -
.../MachO/export-trie-boundary.test | 188 ++++++++++++++++++
4 files changed, 189 insertions(+), 4 deletions(-)
delete mode 100755 llvm/test/tools/llvm-objdump/MachO/Inputs/macho-inconsistent-export
create mode 100644 llvm/test/tools/llvm-objdump/MachO/export-trie-boundary.test
diff --git a/llvm/lib/Object/MachOObjectFile.cpp b/llvm/lib/Object/MachOObjectFile.cpp
index 61d880b19f751..812b2c00ba699 100644
--- a/llvm/lib/Object/MachOObjectFile.cpp
+++ b/llvm/lib/Object/MachOObjectFile.cpp
@@ -3104,7 +3104,7 @@ void ExportEntry::pushNode(uint64_t offset) {
}
}
}
- if(ExportStart + ExportInfoSize != State.Current) {
+ if (ExportStart + ExportInfoSize < State.Current) {
*E = malformedError(
"inconsistent export info size: 0x" +
Twine::utohexstr(ExportInfoSize) + " where actual size was: 0x" +
diff --git a/llvm/test/tools/llvm-objdump/MachO/Inputs/macho-inconsistent-export b/llvm/test/tools/llvm-objdump/MachO/Inputs/macho-inconsistent-export
deleted file mode 100755
index da137800a8d6a025086f900b0473282703f13eca..0000000000000000000000000000000000000000
GIT binary patch
literal 0
HcmV?d00001
literal 8752
zcmeHN&1)M+6n`7pjpDe`h7_89SlKjon=h0&F~#7fSRnzKh9Z9?wGHlg70Fnz(mK-m
zs}K;Rg_>YcZ#|X%0X^jCTTRoHkV{X69D6Awkd|BuA&08JH#;M*HgTc!RG0@(Z{EzC
z-^~2pF6{2p&(DAVYcG+NAfi{GDQLc%=!?czXc4^ueP|I0F?D%*N_KVr*qtsZAD9Ff
zsT?6hP+AMP6-pd$`#{(~r^eauQf+Cvs1bFAs0Yi1db=x*m*#^&dP76(;}~s8Dm*?j
zONb{HOWsncCjHLyo>q7t%4j0?_o%isFYIr*x>~LlMY+1*zXZ=ycsYf^eg|F at Th1>n
ziiMh6DTM>^^KQlP*yo(?CY;wVWnM33r?QbKozGn?=aq4FZiJ|MqP|`!_?}n_YT)tZ
zcp8qckLbRb|LD1e at Z5jGvlWoWV?T<=`_p?vldrqQTFEVn6}QG$OMHKh0y at mW5bcK^
z(IMSXwvAVG0~dX9ykUi>_rQMid<SEAoCm%Jgt#*H at kf_0jp=*LQGp7Nlz&!%@i~#c
zw!DhgaVVc7+V7PM3%;KzuA{-h?+`W4a>F4?!X#mbVMk$ksl2lDu=DVFImW#25b+v!
zjsgI7KQs?z;TTrVC1dh^zngrG`TCxV#=HfG*LiSl>-n+t$ot>@Idtyid&fum(I+X`
z--AVa=%n^xS@{0e=jJ_V!Ep<2ZRgw^$LHoXWSq5zmPKcl>$NioX1#vuRAx|3;rQW5
zyw&pnDea7(T(1YEO6KCuDn##M4)1r{3As3-eCE%HVZbn87%&VN1`Gp+0mHz5XW(jX
z<G0-Ag{u}(>-F0-cXvTbCUxX<3C`nN{J=MNx5*;oZ#}Vd8#kZjZr^<N=m^Ggw|=yv
z<D1?0TCEm(S_4lo+#2{pVqMwn`vr~cWNY9aY%~AKwTba;K0E&CCxquVy1(bqne6n;
z?5AS7HE;*+B9tEgfbsCT{B8CjQOn!o_uU~_R{ieEZGN}u#dD_(RU#X&DJSvS1X*jg
zW%uq)HZk&V`!%SxAE@`pd1ZBf+~Q%_h+)7mU>GnA7zPXjh5^HXVZbn87%&VN1`GrL
z69x_*wI_J1helD^^j1W{_k(&+bC+oRhSMBH-LNPt?V^BUHY($dhxn8s#cJ|AOq;E)
zR=6qU#$g~;T+j2blMJu~>Ve&;K at QQ0Ff`+OWw$O+o;y$H at jFNt0=k%>?LJC$J?*1p
z(h3JDk>qb3dscl>E7g5(1?x}J>^}RObTdr}>strkP5K-gM%MJwi2hamkAUl=2lk<c
z$Hh1Z*Kt!;gPyAKH{o*)lk0UXr=je#WjO~;0Z;!gfop*&6gM4(H=@RE^_^r4KEm6j
z=$X(*S12Opa~VGx&kRO=-aqe$>yVI9?K>obRUbe7$X0*3S`JV;mKhKcT0{Q$caVhS
XULXz=cZiIqTLsIk=mfryeWJes;S at V2
diff --git a/llvm/test/tools/llvm-objdump/MachO/bad-trie.test b/llvm/test/tools/llvm-objdump/MachO/bad-trie.test
index 9567184c7d28d..8b29d30ef0618 100644
--- a/llvm/test/tools/llvm-objdump/MachO/bad-trie.test
+++ b/llvm/test/tools/llvm-objdump/MachO/bad-trie.test
@@ -30,6 +30,3 @@ LOOP_OF_CHILDERN: macho-trie-node-loop': truncated or malformed object (loop in
RUN: not llvm-objdump --macho --exports-trie %p/Inputs/macho-trie-bad-library-ordinal 2>&1 | FileCheck --check-prefix BAD_LIBRARY_ORDINAL %s
BAD_LIBRARY_ORDINAL: macho-trie-bad-library-ordinal': truncated or malformed object (bad library ordinal: 69 (max 3) in export trie data at node: 0x33)
-
-RUN: not llvm-objdump --macho --exports-trie %p/Inputs/macho-inconsistent-export 2>&1 | FileCheck --check-prefix INCONSISTENT_EXPORT_SIZE %s
-INCONSISTENT_EXPORT_SIZE: macho-inconsistent-export': truncated or malformed object (inconsistent export info size: 0x9 where actual size was: 0x5 in export trie data at node: 0x53)
diff --git a/llvm/test/tools/llvm-objdump/MachO/export-trie-boundary.test b/llvm/test/tools/llvm-objdump/MachO/export-trie-boundary.test
new file mode 100644
index 0000000000000..405be70a03db3
--- /dev/null
+++ b/llvm/test/tools/llvm-objdump/MachO/export-trie-boundary.test
@@ -0,0 +1,188 @@
+; RUN: rm -rf %t
+; RUN: split-file %s %t
+; RUN: yaml2obj %t/exports.yaml -o %t/exports.dylib
+; RUN: llvm-objdump --macho --exports-trie %t/exports.dylib | FileCheck %s
+
+; CHECK: Exports trie:
+; CHECK: 0x00003FB0 _foo
+
+# Yaml was manually crafted to have an export trie with a node entry
+# where it's payload is larger than the size needed to read it.
+;--- exports.yaml
+--- !mach-o
+FileHeader:
+ magic: 0xFEEDFACF
+ cputype: 0x100000C
+ cpusubtype: 0x0
+ filetype: 0x6
+ ncmds: 13
+ sizeofcmds: 680
+ flags: 0x100085
+ reserved: 0x0
+LoadCommands:
+ - cmd: LC_SEGMENT_64
+ cmdsize: 232
+ segname: __TEXT
+ vmaddr: 0
+ vmsize: 16384
+ fileoff: 0
+ filesize: 16384
+ maxprot: 5
+ initprot: 5
+ nsects: 2
+ flags: 0
+ Sections:
+ - sectname: __text
+ segname: __TEXT
+ addr: 0x3FB0
+ size: 8
+ offset: 0x3FB0
+ align: 2
+ reloff: 0x0
+ nreloc: 0
+ flags: 0x80000400
+ reserved1: 0x0
+ reserved2: 0x0
+ reserved3: 0x0
+ content: 20008052C0035FD6
+ - sectname: __unwind_info
+ segname: __TEXT
+ addr: 0x3FB8
+ size: 72
+ offset: 0x3FB8
+ align: 2
+ reloff: 0x0
+ nreloc: 0
+ flags: 0x0
+ reserved1: 0x0
+ reserved2: 0x0
+ reserved3: 0x0
+ content: 010000001C000000000000001C000000000000001C00000002000000B03F00003400000034000000B93F00000000000034000000030000000C000100100001000000000000000002
+ - cmd: LC_SEGMENT_64
+ cmdsize: 72
+ segname: __LINKEDIT
+ vmaddr: 16384
+ vmsize: 16384
+ fileoff: 16384
+ filesize: 361
+ maxprot: 1
+ initprot: 1
+ nsects: 0
+ flags: 0
+ - cmd: LC_ID_DYLIB
+ cmdsize: 48
+ dylib:
+ name: 24
+ timestamp: 1
+ current_version: 0
+ compatibility_version: 0
+ Content: '@rpath/libfoo.dylib'
+ ZeroPadBytes: 5
+ - cmd: LC_DYLD_INFO_ONLY
+ cmdsize: 48
+ rebase_off: 0
+ rebase_size: 0
+ bind_off: 0
+ bind_size: 0
+ weak_bind_off: 0
+ weak_bind_size: 0
+ lazy_bind_off: 0
+ lazy_bind_size: 0
+ export_off: 16384
+ export_size: 16
+ - cmd: LC_SYMTAB
+ cmdsize: 24
+ symoff: 16408
+ nsyms: 2
+ stroff: 16440
+ strsize: 24
+ - cmd: LC_DYSYMTAB
+ cmdsize: 80
+ ilocalsym: 0
+ nlocalsym: 0
+ iextdefsym: 0
+ nextdefsym: 1
+ iundefsym: 1
+ nundefsym: 1
+ tocoff: 0
+ ntoc: 0
+ modtaboff: 0
+ nmodtab: 0
+ extrefsymoff: 0
+ nextrefsyms: 0
+ indirectsymoff: 0
+ nindirectsyms: 0
+ extreloff: 0
+ nextrel: 0
+ locreloff: 0
+ nlocrel: 0
+ - cmd: LC_UUID
+ cmdsize: 24
+ uuid: A2CF51D8-828B-3E0F-B8FA-0DF9C5D1C91A
+ - cmd: LC_BUILD_VERSION
+ cmdsize: 32
+ platform: 1
+ minos: 721664
+ sdk: 917504
+ ntools: 1
+ Tools:
+ - tool: 3
+ version: 59441152
+ - cmd: LC_SOURCE_VERSION
+ cmdsize: 16
+ version: 0
+ - cmd: LC_LOAD_DYLIB
+ cmdsize: 56
+ dylib:
+ name: 24
+ timestamp: 2
+ current_version: 87556096
+ compatibility_version: 65536
+ Content: '/usr/lib/libSystem.B.dylib'
+ ZeroPadBytes: 6
+ - cmd: LC_FUNCTION_STARTS
+ cmdsize: 16
+ dataoff: 16400
+ datasize: 8
+ - cmd: LC_DATA_IN_CODE
+ cmdsize: 16
+ dataoff: 16408
+ datasize: 0
+ - cmd: LC_CODE_SIGNATURE
+ cmdsize: 16
+ dataoff: 16464
+ datasize: 281
+LinkEditData:
+ ExportTrie:
+ TerminalSize: 0
+ NodeOffset: 0
+ Name: ''
+ Flags: 0x0
+ Address: 0x0
+ Other: 0x0
+ ImportName: ''
+ Children:
+ - TerminalSize: 4
+ NodeOffset: 8
+ Name: _foo
+ Flags: 0x0
+ Address: 0x3FB0
+ Other: 0x0
+ ImportName: ''
+ NameList:
+ - n_strx: 2
+ n_type: 0xF
+ n_sect: 1
+ n_desc: 0
+ n_value: 16304
+ - n_strx: 7
+ n_type: 0x1
+ n_sect: 0
+ n_desc: 256
+ n_value: 0
+ StringTable:
+ - ' '
+ - _foo
+ - dyld_stub_binder
+ FunctionStarts: [ 0x3FB0 ]
+...
>From 1ee4fa7241d1fcb39c4d183c49765d01039e242e Mon Sep 17 00:00:00 2001
From: Cyndy Ishida <cyndy_ishida at apple.com>
Date: Tue, 25 Jun 2024 19:08:05 -0700
Subject: [PATCH 2/2] Add back testcase to cover failure case
---
.../MachO/export-trie-boundary.test | 193 +++++++++++++++++-
1 file changed, 189 insertions(+), 4 deletions(-)
diff --git a/llvm/test/tools/llvm-objdump/MachO/export-trie-boundary.test b/llvm/test/tools/llvm-objdump/MachO/export-trie-boundary.test
index 405be70a03db3..327003aa7e5ac 100644
--- a/llvm/test/tools/llvm-objdump/MachO/export-trie-boundary.test
+++ b/llvm/test/tools/llvm-objdump/MachO/export-trie-boundary.test
@@ -1,14 +1,18 @@
; RUN: rm -rf %t
; RUN: split-file %s %t
-; RUN: yaml2obj %t/exports.yaml -o %t/exports.dylib
-; RUN: llvm-objdump --macho --exports-trie %t/exports.dylib | FileCheck %s
+; RUN: yaml2obj %t/payload-extended.yaml -o %t/payload-extended.dylib
+; RUN: llvm-objdump --macho --exports-trie %t/payload-extended.dylib 2>&1 | FileCheck %s
-; CHECK: Exports trie:
; CHECK: 0x00003FB0 _foo
+; RUN: yaml2obj %t/payload-truncated.yaml -o %t/payload-truncated.dylib
+; RUN: not llvm-objdump --macho --exports-trie %t/payload-truncated.dylib 2>&1 | FileCheck %s --check-prefix=WRONG_SIZE
+
+; WRONG_SIZE: truncated or malformed object (inconsistent export info size: 0x2 where actual size was: 0x3 in export trie data at node: 0x8)
+
# Yaml was manually crafted to have an export trie with a node entry
# where it's payload is larger than the size needed to read it.
-;--- exports.yaml
+;--- payload-extended.yaml
--- !mach-o
FileHeader:
magic: 0xFEEDFACF
@@ -186,3 +190,184 @@ LinkEditData:
- dyld_stub_binder
FunctionStarts: [ 0x3FB0 ]
...
+
+# Yaml was manually crafted to have an export trie with a node entry
+# where it's payload is smaller than the size needed to read it.
+;--- payload-truncated.yaml
+--- !mach-o
+FileHeader:
+ magic: 0xFEEDFACF
+ cputype: 0x100000C
+ cpusubtype: 0x0
+ filetype: 0x6
+ ncmds: 13
+ sizeofcmds: 680
+ flags: 0x100085
+ reserved: 0x0
+LoadCommands:
+ - cmd: LC_SEGMENT_64
+ cmdsize: 232
+ segname: __TEXT
+ vmaddr: 0
+ vmsize: 16384
+ fileoff: 0
+ filesize: 16384
+ maxprot: 5
+ initprot: 5
+ nsects: 2
+ flags: 0
+ Sections:
+ - sectname: __text
+ segname: __TEXT
+ addr: 0x3FB0
+ size: 8
+ offset: 0x3FB0
+ align: 2
+ reloff: 0x0
+ nreloc: 0
+ flags: 0x80000400
+ reserved1: 0x0
+ reserved2: 0x0
+ reserved3: 0x0
+ content: 20008052C0035FD6
+ - sectname: __unwind_info
+ segname: __TEXT
+ addr: 0x3FB8
+ size: 72
+ offset: 0x3FB8
+ align: 2
+ reloff: 0x0
+ nreloc: 0
+ flags: 0x0
+ reserved1: 0x0
+ reserved2: 0x0
+ reserved3: 0x0
+ content: 010000001C000000000000001C000000000000001C00000002000000B03F00003400000034000000B93F00000000000034000000030000000C000100100001000000000000000002
+ - cmd: LC_SEGMENT_64
+ cmdsize: 72
+ segname: __LINKEDIT
+ vmaddr: 16384
+ vmsize: 16384
+ fileoff: 16384
+ filesize: 361
+ maxprot: 1
+ initprot: 1
+ nsects: 0
+ flags: 0
+ - cmd: LC_ID_DYLIB
+ cmdsize: 48
+ dylib:
+ name: 24
+ timestamp: 1
+ current_version: 0
+ compatibility_version: 0
+ Content: '@rpath/libfoo.dylib'
+ ZeroPadBytes: 5
+ - cmd: LC_DYLD_INFO_ONLY
+ cmdsize: 48
+ rebase_off: 0
+ rebase_size: 0
+ bind_off: 0
+ bind_size: 0
+ weak_bind_off: 0
+ weak_bind_size: 0
+ lazy_bind_off: 0
+ lazy_bind_size: 0
+ export_off: 16384
+ export_size: 16
+ - cmd: LC_SYMTAB
+ cmdsize: 24
+ symoff: 16408
+ nsyms: 2
+ stroff: 16440
+ strsize: 24
+ - cmd: LC_DYSYMTAB
+ cmdsize: 80
+ ilocalsym: 0
+ nlocalsym: 0
+ iextdefsym: 0
+ nextdefsym: 1
+ iundefsym: 1
+ nundefsym: 1
+ tocoff: 0
+ ntoc: 0
+ modtaboff: 0
+ nmodtab: 0
+ extrefsymoff: 0
+ nextrefsyms: 0
+ indirectsymoff: 0
+ nindirectsyms: 0
+ extreloff: 0
+ nextrel: 0
+ locreloff: 0
+ nlocrel: 0
+ - cmd: LC_UUID
+ cmdsize: 24
+ uuid: A2CF51D8-828B-3E0F-B8FA-0DF9C5D1C91A
+ - cmd: LC_BUILD_VERSION
+ cmdsize: 32
+ platform: 1
+ minos: 721664
+ sdk: 917504
+ ntools: 1
+ Tools:
+ - tool: 3
+ version: 59441152
+ - cmd: LC_SOURCE_VERSION
+ cmdsize: 16
+ version: 0
+ - cmd: LC_LOAD_DYLIB
+ cmdsize: 56
+ dylib:
+ name: 24
+ timestamp: 2
+ current_version: 87556096
+ compatibility_version: 65536
+ Content: '/usr/lib/libSystem.B.dylib'
+ ZeroPadBytes: 6
+ - cmd: LC_FUNCTION_STARTS
+ cmdsize: 16
+ dataoff: 16400
+ datasize: 8
+ - cmd: LC_DATA_IN_CODE
+ cmdsize: 16
+ dataoff: 16408
+ datasize: 0
+ - cmd: LC_CODE_SIGNATURE
+ cmdsize: 16
+ dataoff: 16464
+ datasize: 281
+LinkEditData:
+ ExportTrie:
+ TerminalSize: 0
+ NodeOffset: 0
+ Name: ''
+ Flags: 0x0
+ Address: 0x0
+ Other: 0x0
+ ImportName: ''
+ Children:
+ - TerminalSize: 2
+ NodeOffset: 8
+ Name: _foo
+ Flags: 0x0
+ Address: 0x3FB0
+ Other: 0x0
+ ImportName: ''
+ NameList:
+ - n_strx: 2
+ n_type: 0xF
+ n_sect: 1
+ n_desc: 0
+ n_value: 16304
+ - n_strx: 7
+ n_type: 0x1
+ n_sect: 0
+ n_desc: 256
+ n_value: 0
+ StringTable:
+ - ' '
+ - _foo
+ - dyld_stub_binder
+ FunctionStarts: [ 0x3FB0 ]
+...
More information about the llvm-commits
mailing list