Skip to content

Commit

Permalink
[MachO] Loosen boundary check for reading export trie nodes (#96705)
Browse files Browse the repository at this point in the history 
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.
  • Loading branch information
@cyndyishida
cyndyishida committed Jul 9, 2024
1 parent bd7b162 commit cc945e4
Show file tree
Hide file tree
Showing 4 changed files with 374 additions and 4 deletions.
2 changes: 1 addition & 1 deletion llvm/lib/Object/MachOObjectFile.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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" +
Expand Down
Binary file removed llvm/test/tools/llvm-objdump/MachO/Inputs/macho-inconsistent-export
View file
Binary file not shown.
3 changes: 0 additions & 3 deletions llvm/test/tools/llvm-objdump/MachO/bad-trie.test
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
373 changes: 373 additions & 0 deletions llvm/test/tools/llvm-objdump/MachO/export-trie-boundary.test
View file
Original file line number Diff line number Diff line change
@@ -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 ]
...

0 comments on commit cc945e4

Please sign in to comment.