[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 14:47:13 PDT 2024


https://github.com/cyndyishida created https://github.com/llvm/llvm-project/pull/96705

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

This was primarily authored by Nick Kledzik, I added/cleaned up the test cases. 

>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] [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 ]
+...



More information about the llvm-commits mailing list