[MachO] Loosen boundary check for reading export trie nodes (#96705)

cyndyishida · web-flow · commit cc945e41a778 · 2024-07-09T10:18:05.000-07:00
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.
diff --git 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
diff --git 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
@@ -0,0 +1,373 @@
+; RUN: rm -rf %t
+; RUN: split-file %s %t
+; 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: 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.
+;--- payload-extended.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 ]
+...
+
+# 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 ]
+...

Original file line number	Diff line number	Diff line change
`@@ -3104,7 +3104,7 @@ void ExportEntry::pushNode(uint64_t offset) {`
`3104`	`3104`	`}`
`3105`	`3105`	`}`
`3106`	`3106`	`}`
`3107`		`- if(ExportStart + ExportInfoSize != State.Current) {`
	`3107`	`+ if (ExportStart + ExportInfoSize < State.Current) {`
`3108`	`3108`	`*E = malformedError(`
`3109`	`3109`	`"inconsistent export info size: 0x" +`
`3110`	`3110`	`Twine::utohexstr(ExportInfoSize) + " where actual size was: 0x" +`