2018-08-13 18:07:52 +02:00
|
|
|
#!/usr/bin/env python3
|
2023-04-25 13:51:26 +02:00
|
|
|
# Copyright (c) 2015-2020 The Bitcoin Core developers
|
2016-09-19 17:02:23 +02:00
|
|
|
# Distributed under the MIT software license, see the accompanying
|
|
|
|
# file COPYING or http://www.opensource.org/licenses/mit-license.php.
|
2015-10-19 14:53:56 +02:00
|
|
|
'''
|
2020-01-02 12:44:32 +01:00
|
|
|
Perform basic security checks on a series of executables.
|
2016-01-18 10:45:19 +01:00
|
|
|
Exit status will be 0 if successful, and the program will be silent.
|
2015-10-19 14:53:56 +02:00
|
|
|
Otherwise the exit status will be 1 and it will log which executables failed which checks.
|
|
|
|
'''
|
|
|
|
import sys
|
2020-05-14 19:59:54 +02:00
|
|
|
from typing import List, Optional
|
|
|
|
|
2023-05-13 19:44:39 +02:00
|
|
|
import lief
|
2020-11-20 09:15:44 +01:00
|
|
|
import pixie
|
|
|
|
|
2020-05-14 19:59:54 +02:00
|
|
|
def check_ELF_PIE(executable) -> bool:
|
2015-10-19 14:53:56 +02:00
|
|
|
'''
|
|
|
|
Check for position independent executable (PIE), allowing for address space randomization.
|
|
|
|
'''
|
2020-11-20 09:15:44 +01:00
|
|
|
elf = pixie.load(executable)
|
|
|
|
return elf.hdr.e_type == pixie.ET_DYN
|
2015-10-19 14:53:56 +02:00
|
|
|
|
2020-05-14 19:59:54 +02:00
|
|
|
def check_ELF_NX(executable) -> bool:
|
2015-10-19 14:53:56 +02:00
|
|
|
'''
|
|
|
|
Check that no sections are writable and executable (including the stack)
|
|
|
|
'''
|
2020-11-20 09:15:44 +01:00
|
|
|
elf = pixie.load(executable)
|
2015-10-19 14:53:56 +02:00
|
|
|
have_wx = False
|
|
|
|
have_gnu_stack = False
|
2020-11-20 09:15:44 +01:00
|
|
|
for ph in elf.program_headers:
|
|
|
|
if ph.p_type == pixie.PT_GNU_STACK:
|
2015-10-19 14:53:56 +02:00
|
|
|
have_gnu_stack = True
|
2020-11-20 09:15:44 +01:00
|
|
|
if (ph.p_flags & pixie.PF_W) != 0 and (ph.p_flags & pixie.PF_X) != 0: # section is both writable and executable
|
2015-10-19 14:53:56 +02:00
|
|
|
have_wx = True
|
|
|
|
return have_gnu_stack and not have_wx
|
|
|
|
|
2020-05-14 19:59:54 +02:00
|
|
|
def check_ELF_RELRO(executable) -> bool:
|
2015-10-19 14:53:56 +02:00
|
|
|
'''
|
|
|
|
Check for read-only relocations.
|
|
|
|
GNU_RELRO program header must exist
|
|
|
|
Dynamic section must have BIND_NOW flag
|
|
|
|
'''
|
2020-11-20 09:15:44 +01:00
|
|
|
elf = pixie.load(executable)
|
2015-10-19 14:53:56 +02:00
|
|
|
have_gnu_relro = False
|
2020-11-20 09:15:44 +01:00
|
|
|
for ph in elf.program_headers:
|
|
|
|
# Note: not checking p_flags == PF_R: here as linkers set the permission differently
|
2020-05-14 19:59:54 +02:00
|
|
|
# This does not affect security: the permission flags of the GNU_RELRO program
|
|
|
|
# header are ignored, the PT_LOAD header determines the effective permissions.
|
2015-10-19 14:53:56 +02:00
|
|
|
# However, the dynamic linker need to write to this area so these are RW.
|
|
|
|
# Glibc itself takes care of mprotecting this area R after relocations are finished.
|
2018-12-04 11:46:21 +01:00
|
|
|
# See also https://marc.info/?l=binutils&m=1498883354122353
|
2020-11-20 09:15:44 +01:00
|
|
|
if ph.p_type == pixie.PT_GNU_RELRO:
|
2015-10-19 14:53:56 +02:00
|
|
|
have_gnu_relro = True
|
|
|
|
|
|
|
|
have_bindnow = False
|
2020-11-20 09:15:44 +01:00
|
|
|
for flags in elf.query_dyn_tags(pixie.DT_FLAGS):
|
|
|
|
assert isinstance(flags, int)
|
|
|
|
if flags & pixie.DF_BIND_NOW:
|
2015-10-19 14:53:56 +02:00
|
|
|
have_bindnow = True
|
2020-11-20 09:15:44 +01:00
|
|
|
|
2015-10-19 14:53:56 +02:00
|
|
|
return have_gnu_relro and have_bindnow
|
|
|
|
|
2020-05-14 19:59:54 +02:00
|
|
|
def check_ELF_Canary(executable) -> bool:
|
2015-10-19 14:53:56 +02:00
|
|
|
'''
|
|
|
|
Check for use of stack canary
|
|
|
|
'''
|
2020-11-20 09:15:44 +01:00
|
|
|
elf = pixie.load(executable)
|
2015-10-19 14:53:56 +02:00
|
|
|
ok = False
|
2020-11-20 09:15:44 +01:00
|
|
|
for symbol in elf.dyn_symbols:
|
|
|
|
if symbol.name == b'__stack_chk_fail':
|
2015-10-19 14:53:56 +02:00
|
|
|
ok = True
|
|
|
|
return ok
|
|
|
|
|
2020-07-10 14:53:28 +02:00
|
|
|
def check_ELF_separate_code(executable):
|
|
|
|
'''
|
|
|
|
Check that sections are appropriately separated in virtual memory,
|
|
|
|
based on their permissions. This checks for missing -Wl,-z,separate-code
|
|
|
|
and potentially other problems.
|
|
|
|
'''
|
2020-11-20 09:15:44 +01:00
|
|
|
elf = pixie.load(executable)
|
|
|
|
R = pixie.PF_R
|
|
|
|
W = pixie.PF_W
|
|
|
|
E = pixie.PF_X
|
2020-07-10 14:53:28 +02:00
|
|
|
EXPECTED_FLAGS = {
|
|
|
|
# Read + execute
|
2020-11-20 09:15:44 +01:00
|
|
|
b'.init': R | E,
|
|
|
|
b'.plt': R | E,
|
|
|
|
b'.plt.got': R | E,
|
|
|
|
b'.plt.sec': R | E,
|
|
|
|
b'.text': R | E,
|
|
|
|
b'.fini': R | E,
|
2020-07-10 14:53:28 +02:00
|
|
|
# Read-only data
|
2020-11-20 09:15:44 +01:00
|
|
|
b'.interp': R,
|
|
|
|
b'.note.gnu.property': R,
|
|
|
|
b'.note.gnu.build-id': R,
|
|
|
|
b'.note.ABI-tag': R,
|
|
|
|
b'.gnu.hash': R,
|
|
|
|
b'.dynsym': R,
|
|
|
|
b'.dynstr': R,
|
|
|
|
b'.gnu.version': R,
|
|
|
|
b'.gnu.version_r': R,
|
|
|
|
b'.rela.dyn': R,
|
|
|
|
b'.rela.plt': R,
|
|
|
|
b'.rodata': R,
|
|
|
|
b'.eh_frame_hdr': R,
|
|
|
|
b'.eh_frame': R,
|
|
|
|
b'.qtmetadata': R,
|
|
|
|
b'.gcc_except_table': R,
|
|
|
|
b'.stapsdt.base': R,
|
2020-07-10 14:53:28 +02:00
|
|
|
# Writable data
|
2020-11-20 09:15:44 +01:00
|
|
|
b'.init_array': R | W,
|
|
|
|
b'.fini_array': R | W,
|
|
|
|
b'.dynamic': R | W,
|
|
|
|
b'.got': R | W,
|
|
|
|
b'.data': R | W,
|
|
|
|
b'.bss': R | W,
|
2020-07-10 14:53:28 +02:00
|
|
|
}
|
2020-11-20 09:15:44 +01:00
|
|
|
if elf.hdr.e_machine == pixie.EM_PPC64:
|
|
|
|
# .plt is RW on ppc64 even with separate-code
|
|
|
|
EXPECTED_FLAGS[b'.plt'] = R | W
|
2020-07-10 14:53:28 +02:00
|
|
|
# For all LOAD program headers get mapping to the list of sections,
|
|
|
|
# and for each section, remember the flags of the associated program header.
|
|
|
|
flags_per_section = {}
|
2020-11-20 09:15:44 +01:00
|
|
|
for ph in elf.program_headers:
|
|
|
|
if ph.p_type == pixie.PT_LOAD:
|
|
|
|
for section in ph.sections:
|
|
|
|
assert(section.name not in flags_per_section)
|
|
|
|
flags_per_section[section.name] = ph.p_flags
|
2020-07-10 14:53:28 +02:00
|
|
|
# Spot-check ELF LOAD program header flags per section
|
|
|
|
# If these sections exist, check them against the expected R/W/E flags
|
|
|
|
for (section, flags) in flags_per_section.items():
|
|
|
|
if section in EXPECTED_FLAGS:
|
|
|
|
if EXPECTED_FLAGS[section] != flags:
|
|
|
|
return False
|
|
|
|
return True
|
|
|
|
|
2020-05-14 19:59:54 +02:00
|
|
|
def check_PE_DYNAMIC_BASE(executable) -> bool:
|
2015-10-19 14:53:56 +02:00
|
|
|
'''PIE: DllCharacteristics bit 0x40 signifies dynamicbase (ASLR)'''
|
2023-05-13 19:44:39 +02:00
|
|
|
binary = lief.parse(executable)
|
|
|
|
return lief.PE.DLL_CHARACTERISTICS.DYNAMIC_BASE in binary.optional_header.dll_characteristics_lists
|
2016-09-26 13:03:44 +02:00
|
|
|
|
2020-05-14 19:59:54 +02:00
|
|
|
# Must support high-entropy 64-bit address space layout randomization
|
|
|
|
# in addition to DYNAMIC_BASE to have secure ASLR.
|
|
|
|
def check_PE_HIGH_ENTROPY_VA(executable) -> bool:
|
2016-09-26 13:03:44 +02:00
|
|
|
'''PIE: DllCharacteristics bit 0x20 signifies high-entropy ASLR'''
|
2023-05-13 19:44:39 +02:00
|
|
|
binary = lief.parse(executable)
|
|
|
|
return lief.PE.DLL_CHARACTERISTICS.HIGH_ENTROPY_VA in binary.optional_header.dll_characteristics_lists
|
2015-10-19 14:53:56 +02:00
|
|
|
|
Merge #18629: scripts: add PE .reloc section check to security-check.py
3e38023af724a76972d39cbccfb0bba4c54a0323 scripts: add PE .reloc section check to security-check.py (fanquake)
Pull request description:
The `ld` in binutils has historically had a few issues with PE binaries, there's a good summary in this [thread](https://sourceware.org/bugzilla/show_bug.cgi?id=19011).
One issue in particular was `ld` stripping the `.reloc` section out of PE binaries, even though it's required for functioning ASLR. This was [reported by a Tor developer in 2014](https://sourceware.org/bugzilla/show_bug.cgi?id=17321) and they have been patching their [own binutils](https://gitweb.torproject.org/builders/tor-browser-build.git/tree/projects/binutils) ever since. However their patch only made it into binutils at the [start of this year](https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;a=commit;h=dc9bd8c92af67947db44b3cb428c050259b15cd0). It adds an `--enable-reloc-section` flag, which is turned on by default if you are using `--dynamic-base`. In the mean time this issue has also been worked around by other projects, such as FFmpeg, see [this commit](https://github.com/TheRyuu/FFmpeg/commit/91b668acd6decec0a6f8d20bf56e2644f96adcb9).
I have checked our recent supported Windows release binaries, and they do contain a `.reloc` section. From what I understand, we are using all the right compile/linker flags, including `-pie` & `-fPIE`, and have never run into the crashing/entrypoint issues that other projects might have seen.
One other thing worth noting here, it how Debian/Ubuntu patch the binutils that they distribute, because that's what we end up using in our gitian builds.
In the binutils-mingw-w64 in Bionic (18.04), which we currently use in gitian, PE hardening options/security flags are enabled by default. See the [changelog](https://changelogs.ubuntu.com/changelogs/pool/universe/b/binutils-mingw-w64/binutils-mingw-w64_8ubuntu1/changelog) and the [relevant commit](https://salsa.debian.org/mingw-w64-team/binutils-mingw-w64/-/commit/452b3013b8280cbe35eaeb166a43621b88d5f8b7).
However in Focal (20.04), this has now been reversed. PE hardening options are no-longer the default. See the [changelog](https://changelogs.ubuntu.com/changelogs/pool/universe/b/binutils-mingw-w64/binutils-mingw-w64_8.8/changelog) and [relevant commit](https://salsa.debian.org/mingw-w64-team/binutils-mingw-w64/-/commit/7bd8b2fbc242a8c2fc2217f29fd61f94d3babf6f), which cites same .reloc issue mentioned here.
Given that we explicitly specify/opt-in to everything that we want to use, the defaults aren't necessarily an issue for us. However I think it highlights the importance of continuing to be explicit about what we want, and not falling-back or relying on upstream.
This was also prompted by the possibility of us doing link time garbage collection, see #18579 & #18605. It seemed some sanity checks would be worthwhile in-case the linker goes haywire while garbage collecting.
I think Guix is going to bring great benefits when dealing with these kinds of issues. Carl you might have something to say in that regard.
ACKs for top commit:
dongcarl:
ACK 3e38023af724a76972d39cbccfb0bba4c54a0323
Tree-SHA512: af14d63bdb334bde548dd7de3e0946556b7e2598d817b56eb4e75b3f56c705c26aa85dd9783134c4b6a7aeb7cb4de567eed996e94d533d31511f57ed332287da
2020-04-28 07:08:19 +02:00
|
|
|
def check_PE_RELOC_SECTION(executable) -> bool:
|
|
|
|
'''Check for a reloc section. This is required for functional ASLR.'''
|
2023-05-13 19:44:39 +02:00
|
|
|
binary = lief.parse(executable)
|
|
|
|
return binary.has_relocations
|
2020-01-02 12:44:32 +01:00
|
|
|
|
|
|
|
def check_MACHO_NOUNDEFS(executable) -> bool:
|
|
|
|
'''
|
|
|
|
Check for no undefined references.
|
|
|
|
'''
|
2023-05-13 19:44:39 +02:00
|
|
|
binary = lief.parse(executable)
|
|
|
|
return binary.header.has(lief.MachO.HEADER_FLAGS.NOUNDEFS)
|
2020-03-28 11:48:19 +01:00
|
|
|
|
Merge #18295: scripts: add MACHO lazy bindings check to security-check.py
5ca90f8b598978437340bb8467f527b9edfb2bbf scripts: add MACHO lazy bindings check to security-check.py (fanquake)
Pull request description:
This is a slightly belated follow up to #17686 and some discussion with Cory. It's not entirely clear if we should make this change due to the way the macOS dynamic loader appears to work. However I'm opening this for some discussion. Also related to #17768.
#### Issue:
[`LD64`](https://opensource.apple.com/source/ld64/) doesn't set the [MH_BINDATLOAD](https://opensource.apple.com/source/xnu/xnu-6153.11.26/EXTERNAL_HEADERS/mach-o/loader.h.auto.html) bit in the header of MACHO executables, when building with `-bind_at_load`. This is in contradiction to the [documentation](https://opensource.apple.com/source/ld64/ld64-450.3/doc/man/man1/ld.1.auto.html):
```bash
-bind_at_load
Sets a bit in the mach header of the resulting binary which tells dyld to
bind all symbols when the binary is loaded, rather than lazily.
```
The [`ld` in Apples cctools](https://opensource.apple.com/source/cctools/cctools-927.0.2/ld/layout.c.auto.html) does set the bit, however the [cctools-port](https://github.com/tpoechtrager/cctools-port/) that we use for release builds, bundles `LD64`.
However; even if the linker hasn't set that bit, the dynamic loader ([`dyld`](https://opensource.apple.com/source/dyld/)) doesn't seem to ever check for it, and from what I understand, it looks at a different part of the header when determining whether to lazily load symbols.
Note that our release binaries are currently working as expected, and no lazy loading occurs.
#### Example:
Using a small program, we can observe the behaviour of the dynamic loader.
Conducted using:
```bash
clang++ --version
Apple clang version 11.0.0 (clang-1100.0.33.17)
Target: x86_64-apple-darwin18.7.0
ld -v
@(#)PROGRAM:ld PROJECT:ld64-530
BUILD 18:57:17 Dec 13 2019
LTO support using: LLVM version 11.0.0, (clang-1100.0.33.17) (static support for 23, runtime is 23)
TAPI support using: Apple TAPI version 11.0.0 (tapi-1100.0.11)
```
```cpp
#include <iostream>
int main() {
std::cout << "Hello World!\n";
return 0;
}
```
Compile and check the MACHO header:
```bash
clang++ test.cpp -o test
otool -vh test
...
Mach header
magic cputype cpusubtype caps filetype ncmds sizeofcmds flags
MH_MAGIC_64 X86_64 ALL LIB64 EXECUTE 16 1424 NOUNDEFS DYLDLINK TWOLEVEL WEAK_DEFINES BINDS_TO_WEAK PIE
# Run and dump dynamic loader bindings:
DYLD_PRINT_BINDINGS=1 DYLD_PRINT_TO_FILE=no_bind.txt ./test
Hello World!
```
Recompile with `-bind_at_load`. Note still no `BINDATLOAD` flag:
```bash
clang++ test.cpp -o test -Wl,-bind_at_load
otool -vh test
Mach header
magic cputype cpusubtype caps filetype ncmds sizeofcmds flags
MH_MAGIC_64 X86_64 ALL LIB64 EXECUTE 16 1424 NOUNDEFS DYLDLINK TWOLEVEL WEAK_DEFINES BINDS_TO_WEAK PIE
...
DYLD_PRINT_BINDINGS=1 DYLD_PRINT_TO_FILE=bind.txt ./test
Hello World!
```
If we diff the outputs, you can see that `dyld` doesn't perform any lazy bindings when the binary is compiled with `-bind_at_load`, even if the `BINDATLOAD` flag is not set:
```diff
@@ -1,11 +1,27 @@
+dyld: bind: test:0x103EDF030 = libc++.1.dylib:__ZNKSt3__16locale9use_facetERNS0_2idE, *0x103EDF030 = 0x7FFF70C9FA58
+dyld: bind: test:0x103EDF038 = libc++.1.dylib:__ZNKSt3__18ios_base6getlocEv, *0x103EDF038 = 0x7FFF70CA12C2
+dyld: bind: test:0x103EDF068 = libc++.1.dylib:__ZNSt3__113basic_ostreamIcNS_11char_traitsIcEEE6sentryC1ERS3_, *0x103EDF068 = 0x7FFF70CA12B6
+dyld: bind: test:0x103EDF070 = libc++.1.dylib:__ZNSt3__113basic_ostreamIcNS_11char_traitsIcEEE6sentryD1Ev, *0x103EDF070 = 0x7FFF70CA1528
+dyld: bind: test:0x103EDF080 = libc++.1.dylib:__ZNSt3__16localeD1Ev, *0x103EDF080 = 0x7FFF70C9FAE6
<trim>
-dyld: lazy bind: test:0x10D4AC0C8 = libsystem_platform.dylib:_strlen, *0x10D4AC0C8 = 0x7FFF73C5C6E0
-dyld: lazy bind: test:0x10D4AC068 = libc++.1.dylib:__ZNSt3__113basic_ostreamIcNS_11char_traitsIcEEE6sentryC1ERS3_, *0x10D4AC068 = 0x7FFF70CA12B6
-dyld: lazy bind: test:0x10D4AC038 = libc++.1.dylib:__ZNKSt3__18ios_base6getlocEv, *0x10D4AC038 = 0x7FFF70CA12C2
-dyld: lazy bind: test:0x10D4AC030 = libc++.1.dylib:__ZNKSt3__16locale9use_facetERNS0_2idE, *0x10D4AC030 = 0x7FFF70C9FA58
-dyld: lazy bind: test:0x10D4AC080 = libc++.1.dylib:__ZNSt3__16localeD1Ev, *0x10D4AC080 = 0x7FFF70C9FAE6
-dyld: lazy bind: test:0x10D4AC070 = libc++.1.dylib:__ZNSt3__113basic_ostreamIcNS_11char_traitsIcEEE6sentryD1Ev, *0x10D4AC070 = 0x7FFF70CA1528
```
Note: `dyld` also has a `DYLD_BIND_AT_LAUNCH=1` environment variable, that when set, will force any lazy bindings to be non-lazy:
```bash
dyld: forced lazy bind: test:0x10BEC8068 = libc++.1.dylib:__ZNSt3__113basic_ostream
```
#### Thoughts:
After looking at the dyld source, I can't find any checks for `MH_BINDATLOAD`. You can see the flags it does check for, such as MH_PIE or MH_BIND_TO_WEAK [here](https://opensource.apple.com/source/dyld/dyld-732.8/src/ImageLoaderMachO.cpp.auto.html).
It seems that the lazy binding of any symbols depends on whether or not [lazy_bind_size](https://opensource.apple.com/source/xnu/xnu-6153.11.26/EXTERNAL_HEADERS/mach-o/loader.h.auto.html) from the `LC_DYLD_INFO_ONLY` load command is > 0. Which was mentioned in [#17686](https://github.com/bitcoin/bitcoin/pull/17686#issue-350216254).
#### Changes:
This PR is one of [Corys commits](https://github.com/theuni/bitcoin/commit/7b6ba26178d2754568a1308d3d44e038e9ebf450), that I've rebased and modified to make build. I've also included an addition to the `security-check.py` script to check for the flag.
However, given the above, I'm not entirely sure this patch is the correct approach. If the linker no-longer inserts it, and the dynamic loader doesn't look for it, there might be little benefit to setting it. Or, maybe this is an oversight from Apple and needs some upstream discussion. Looking for some thoughts / Concept ACK/NACK.
One alternate approach we could take is to drop the patch and modify security-check.py to look for `lazy_bind_size` == 0 in the `LC_DYLD_INFO_ONLY` load command, using `otool -l`.
ACKs for top commit:
theuni:
ACK 5ca90f8b598978437340bb8467f527b9edfb2bbf
Tree-SHA512: 444022ea9d19ed74dd06dc2ab3857a9c23fbc2f6475364e8552d761b712d684b3a7114d144f20de42328d1a99403b48667ba96885121392affb2e05b834b6e1c
2020-04-10 01:10:01 +02:00
|
|
|
def check_MACHO_LAZY_BINDINGS(executable) -> bool:
|
|
|
|
'''
|
|
|
|
Check for no lazy bindings.
|
|
|
|
We don't use or check for MH_BINDATLOAD. See #18295.
|
|
|
|
'''
|
2023-05-13 19:44:39 +02:00
|
|
|
binary = lief.parse(executable)
|
|
|
|
return binary.dyld_info.lazy_bind == (0,0)
|
Merge #18295: scripts: add MACHO lazy bindings check to security-check.py
5ca90f8b598978437340bb8467f527b9edfb2bbf scripts: add MACHO lazy bindings check to security-check.py (fanquake)
Pull request description:
This is a slightly belated follow up to #17686 and some discussion with Cory. It's not entirely clear if we should make this change due to the way the macOS dynamic loader appears to work. However I'm opening this for some discussion. Also related to #17768.
#### Issue:
[`LD64`](https://opensource.apple.com/source/ld64/) doesn't set the [MH_BINDATLOAD](https://opensource.apple.com/source/xnu/xnu-6153.11.26/EXTERNAL_HEADERS/mach-o/loader.h.auto.html) bit in the header of MACHO executables, when building with `-bind_at_load`. This is in contradiction to the [documentation](https://opensource.apple.com/source/ld64/ld64-450.3/doc/man/man1/ld.1.auto.html):
```bash
-bind_at_load
Sets a bit in the mach header of the resulting binary which tells dyld to
bind all symbols when the binary is loaded, rather than lazily.
```
The [`ld` in Apples cctools](https://opensource.apple.com/source/cctools/cctools-927.0.2/ld/layout.c.auto.html) does set the bit, however the [cctools-port](https://github.com/tpoechtrager/cctools-port/) that we use for release builds, bundles `LD64`.
However; even if the linker hasn't set that bit, the dynamic loader ([`dyld`](https://opensource.apple.com/source/dyld/)) doesn't seem to ever check for it, and from what I understand, it looks at a different part of the header when determining whether to lazily load symbols.
Note that our release binaries are currently working as expected, and no lazy loading occurs.
#### Example:
Using a small program, we can observe the behaviour of the dynamic loader.
Conducted using:
```bash
clang++ --version
Apple clang version 11.0.0 (clang-1100.0.33.17)
Target: x86_64-apple-darwin18.7.0
ld -v
@(#)PROGRAM:ld PROJECT:ld64-530
BUILD 18:57:17 Dec 13 2019
LTO support using: LLVM version 11.0.0, (clang-1100.0.33.17) (static support for 23, runtime is 23)
TAPI support using: Apple TAPI version 11.0.0 (tapi-1100.0.11)
```
```cpp
#include <iostream>
int main() {
std::cout << "Hello World!\n";
return 0;
}
```
Compile and check the MACHO header:
```bash
clang++ test.cpp -o test
otool -vh test
...
Mach header
magic cputype cpusubtype caps filetype ncmds sizeofcmds flags
MH_MAGIC_64 X86_64 ALL LIB64 EXECUTE 16 1424 NOUNDEFS DYLDLINK TWOLEVEL WEAK_DEFINES BINDS_TO_WEAK PIE
# Run and dump dynamic loader bindings:
DYLD_PRINT_BINDINGS=1 DYLD_PRINT_TO_FILE=no_bind.txt ./test
Hello World!
```
Recompile with `-bind_at_load`. Note still no `BINDATLOAD` flag:
```bash
clang++ test.cpp -o test -Wl,-bind_at_load
otool -vh test
Mach header
magic cputype cpusubtype caps filetype ncmds sizeofcmds flags
MH_MAGIC_64 X86_64 ALL LIB64 EXECUTE 16 1424 NOUNDEFS DYLDLINK TWOLEVEL WEAK_DEFINES BINDS_TO_WEAK PIE
...
DYLD_PRINT_BINDINGS=1 DYLD_PRINT_TO_FILE=bind.txt ./test
Hello World!
```
If we diff the outputs, you can see that `dyld` doesn't perform any lazy bindings when the binary is compiled with `-bind_at_load`, even if the `BINDATLOAD` flag is not set:
```diff
@@ -1,11 +1,27 @@
+dyld: bind: test:0x103EDF030 = libc++.1.dylib:__ZNKSt3__16locale9use_facetERNS0_2idE, *0x103EDF030 = 0x7FFF70C9FA58
+dyld: bind: test:0x103EDF038 = libc++.1.dylib:__ZNKSt3__18ios_base6getlocEv, *0x103EDF038 = 0x7FFF70CA12C2
+dyld: bind: test:0x103EDF068 = libc++.1.dylib:__ZNSt3__113basic_ostreamIcNS_11char_traitsIcEEE6sentryC1ERS3_, *0x103EDF068 = 0x7FFF70CA12B6
+dyld: bind: test:0x103EDF070 = libc++.1.dylib:__ZNSt3__113basic_ostreamIcNS_11char_traitsIcEEE6sentryD1Ev, *0x103EDF070 = 0x7FFF70CA1528
+dyld: bind: test:0x103EDF080 = libc++.1.dylib:__ZNSt3__16localeD1Ev, *0x103EDF080 = 0x7FFF70C9FAE6
<trim>
-dyld: lazy bind: test:0x10D4AC0C8 = libsystem_platform.dylib:_strlen, *0x10D4AC0C8 = 0x7FFF73C5C6E0
-dyld: lazy bind: test:0x10D4AC068 = libc++.1.dylib:__ZNSt3__113basic_ostreamIcNS_11char_traitsIcEEE6sentryC1ERS3_, *0x10D4AC068 = 0x7FFF70CA12B6
-dyld: lazy bind: test:0x10D4AC038 = libc++.1.dylib:__ZNKSt3__18ios_base6getlocEv, *0x10D4AC038 = 0x7FFF70CA12C2
-dyld: lazy bind: test:0x10D4AC030 = libc++.1.dylib:__ZNKSt3__16locale9use_facetERNS0_2idE, *0x10D4AC030 = 0x7FFF70C9FA58
-dyld: lazy bind: test:0x10D4AC080 = libc++.1.dylib:__ZNSt3__16localeD1Ev, *0x10D4AC080 = 0x7FFF70C9FAE6
-dyld: lazy bind: test:0x10D4AC070 = libc++.1.dylib:__ZNSt3__113basic_ostreamIcNS_11char_traitsIcEEE6sentryD1Ev, *0x10D4AC070 = 0x7FFF70CA1528
```
Note: `dyld` also has a `DYLD_BIND_AT_LAUNCH=1` environment variable, that when set, will force any lazy bindings to be non-lazy:
```bash
dyld: forced lazy bind: test:0x10BEC8068 = libc++.1.dylib:__ZNSt3__113basic_ostream
```
#### Thoughts:
After looking at the dyld source, I can't find any checks for `MH_BINDATLOAD`. You can see the flags it does check for, such as MH_PIE or MH_BIND_TO_WEAK [here](https://opensource.apple.com/source/dyld/dyld-732.8/src/ImageLoaderMachO.cpp.auto.html).
It seems that the lazy binding of any symbols depends on whether or not [lazy_bind_size](https://opensource.apple.com/source/xnu/xnu-6153.11.26/EXTERNAL_HEADERS/mach-o/loader.h.auto.html) from the `LC_DYLD_INFO_ONLY` load command is > 0. Which was mentioned in [#17686](https://github.com/bitcoin/bitcoin/pull/17686#issue-350216254).
#### Changes:
This PR is one of [Corys commits](https://github.com/theuni/bitcoin/commit/7b6ba26178d2754568a1308d3d44e038e9ebf450), that I've rebased and modified to make build. I've also included an addition to the `security-check.py` script to check for the flag.
However, given the above, I'm not entirely sure this patch is the correct approach. If the linker no-longer inserts it, and the dynamic loader doesn't look for it, there might be little benefit to setting it. Or, maybe this is an oversight from Apple and needs some upstream discussion. Looking for some thoughts / Concept ACK/NACK.
One alternate approach we could take is to drop the patch and modify security-check.py to look for `lazy_bind_size` == 0 in the `LC_DYLD_INFO_ONLY` load command, using `otool -l`.
ACKs for top commit:
theuni:
ACK 5ca90f8b598978437340bb8467f527b9edfb2bbf
Tree-SHA512: 444022ea9d19ed74dd06dc2ab3857a9c23fbc2f6475364e8552d761b712d684b3a7114d144f20de42328d1a99403b48667ba96885121392affb2e05b834b6e1c
2020-04-10 01:10:01 +02:00
|
|
|
|
2020-04-22 10:19:51 +02:00
|
|
|
def check_MACHO_Canary(executable) -> bool:
|
|
|
|
'''
|
|
|
|
Check for use of stack canary
|
|
|
|
'''
|
2023-05-13 19:44:39 +02:00
|
|
|
binary = lief.parse(executable)
|
|
|
|
return binary.has_symbol('___stack_chk_fail')
|
2020-05-14 19:59:54 +02:00
|
|
|
|
2023-05-13 19:44:39 +02:00
|
|
|
def check_PIE(executable) -> bool:
|
|
|
|
'''
|
|
|
|
Check for position independent executable (PIE),
|
|
|
|
allowing for address space randomization.
|
|
|
|
'''
|
|
|
|
binary = lief.parse(executable)
|
|
|
|
return binary.is_pie
|
|
|
|
|
|
|
|
def check_NX(executable) -> bool:
|
|
|
|
'''
|
|
|
|
Check for no stack execution
|
|
|
|
'''
|
|
|
|
binary = lief.parse(executable)
|
|
|
|
return binary.has_nx
|
2020-04-22 10:19:51 +02:00
|
|
|
|
2015-10-19 14:53:56 +02:00
|
|
|
CHECKS = {
|
|
|
|
'ELF': [
|
|
|
|
('PIE', check_ELF_PIE),
|
|
|
|
('NX', check_ELF_NX),
|
|
|
|
('RELRO', check_ELF_RELRO),
|
2020-07-10 14:53:28 +02:00
|
|
|
('Canary', check_ELF_Canary),
|
|
|
|
('separate_code', check_ELF_separate_code),
|
2015-10-19 14:53:56 +02:00
|
|
|
],
|
|
|
|
'PE': [
|
2023-05-13 19:44:39 +02:00
|
|
|
('PIE', check_PIE),
|
2016-09-26 13:03:44 +02:00
|
|
|
('DYNAMIC_BASE', check_PE_DYNAMIC_BASE),
|
|
|
|
('HIGH_ENTROPY_VA', check_PE_HIGH_ENTROPY_VA),
|
2023-05-13 19:44:39 +02:00
|
|
|
('NX', check_NX),
|
Merge #18629: scripts: add PE .reloc section check to security-check.py
3e38023af724a76972d39cbccfb0bba4c54a0323 scripts: add PE .reloc section check to security-check.py (fanquake)
Pull request description:
The `ld` in binutils has historically had a few issues with PE binaries, there's a good summary in this [thread](https://sourceware.org/bugzilla/show_bug.cgi?id=19011).
One issue in particular was `ld` stripping the `.reloc` section out of PE binaries, even though it's required for functioning ASLR. This was [reported by a Tor developer in 2014](https://sourceware.org/bugzilla/show_bug.cgi?id=17321) and they have been patching their [own binutils](https://gitweb.torproject.org/builders/tor-browser-build.git/tree/projects/binutils) ever since. However their patch only made it into binutils at the [start of this year](https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;a=commit;h=dc9bd8c92af67947db44b3cb428c050259b15cd0). It adds an `--enable-reloc-section` flag, which is turned on by default if you are using `--dynamic-base`. In the mean time this issue has also been worked around by other projects, such as FFmpeg, see [this commit](https://github.com/TheRyuu/FFmpeg/commit/91b668acd6decec0a6f8d20bf56e2644f96adcb9).
I have checked our recent supported Windows release binaries, and they do contain a `.reloc` section. From what I understand, we are using all the right compile/linker flags, including `-pie` & `-fPIE`, and have never run into the crashing/entrypoint issues that other projects might have seen.
One other thing worth noting here, it how Debian/Ubuntu patch the binutils that they distribute, because that's what we end up using in our gitian builds.
In the binutils-mingw-w64 in Bionic (18.04), which we currently use in gitian, PE hardening options/security flags are enabled by default. See the [changelog](https://changelogs.ubuntu.com/changelogs/pool/universe/b/binutils-mingw-w64/binutils-mingw-w64_8ubuntu1/changelog) and the [relevant commit](https://salsa.debian.org/mingw-w64-team/binutils-mingw-w64/-/commit/452b3013b8280cbe35eaeb166a43621b88d5f8b7).
However in Focal (20.04), this has now been reversed. PE hardening options are no-longer the default. See the [changelog](https://changelogs.ubuntu.com/changelogs/pool/universe/b/binutils-mingw-w64/binutils-mingw-w64_8.8/changelog) and [relevant commit](https://salsa.debian.org/mingw-w64-team/binutils-mingw-w64/-/commit/7bd8b2fbc242a8c2fc2217f29fd61f94d3babf6f), which cites same .reloc issue mentioned here.
Given that we explicitly specify/opt-in to everything that we want to use, the defaults aren't necessarily an issue for us. However I think it highlights the importance of continuing to be explicit about what we want, and not falling-back or relying on upstream.
This was also prompted by the possibility of us doing link time garbage collection, see #18579 & #18605. It seemed some sanity checks would be worthwhile in-case the linker goes haywire while garbage collecting.
I think Guix is going to bring great benefits when dealing with these kinds of issues. Carl you might have something to say in that regard.
ACKs for top commit:
dongcarl:
ACK 3e38023af724a76972d39cbccfb0bba4c54a0323
Tree-SHA512: af14d63bdb334bde548dd7de3e0946556b7e2598d817b56eb4e75b3f56c705c26aa85dd9783134c4b6a7aeb7cb4de567eed996e94d533d31511f57ed332287da
2020-04-28 07:08:19 +02:00
|
|
|
('RELOC_SECTION', check_PE_RELOC_SECTION)
|
2020-01-02 12:44:32 +01:00
|
|
|
],
|
|
|
|
'MACHO': [
|
2023-05-13 19:44:39 +02:00
|
|
|
('PIE', check_PIE),
|
2020-01-02 12:44:32 +01:00
|
|
|
('NOUNDEFS', check_MACHO_NOUNDEFS),
|
2023-05-13 19:44:39 +02:00
|
|
|
('NX', check_NX),
|
2020-04-22 10:19:51 +02:00
|
|
|
('LAZY_BINDINGS', check_MACHO_LAZY_BINDINGS),
|
|
|
|
('Canary', check_MACHO_Canary)
|
2015-10-19 14:53:56 +02:00
|
|
|
]
|
|
|
|
}
|
|
|
|
|
2020-05-14 19:59:54 +02:00
|
|
|
def identify_executable(executable) -> Optional[str]:
|
2015-10-19 14:53:56 +02:00
|
|
|
with open(filename, 'rb') as f:
|
|
|
|
magic = f.read(4)
|
|
|
|
if magic.startswith(b'MZ'):
|
|
|
|
return 'PE'
|
|
|
|
elif magic.startswith(b'\x7fELF'):
|
|
|
|
return 'ELF'
|
2020-01-02 12:44:32 +01:00
|
|
|
elif magic.startswith(b'\xcf\xfa'):
|
|
|
|
return 'MACHO'
|
2015-10-19 14:53:56 +02:00
|
|
|
return None
|
|
|
|
|
|
|
|
if __name__ == '__main__':
|
2023-05-13 19:44:39 +02:00
|
|
|
retval: int = 0
|
2015-10-19 14:53:56 +02:00
|
|
|
for filename in sys.argv[1:]:
|
|
|
|
try:
|
|
|
|
etype = identify_executable(filename)
|
|
|
|
if etype is None:
|
2023-05-13 19:44:39 +02:00
|
|
|
print(f'{filename}: unknown format')
|
2015-10-19 14:53:56 +02:00
|
|
|
retval = 1
|
|
|
|
continue
|
|
|
|
|
2023-05-13 19:44:39 +02:00
|
|
|
failed: List[str] = []
|
2015-10-19 14:53:56 +02:00
|
|
|
for (name, func) in CHECKS[etype]:
|
|
|
|
if not func(filename):
|
2020-05-14 19:59:54 +02:00
|
|
|
failed.append(name)
|
2015-10-19 14:53:56 +02:00
|
|
|
if failed:
|
2023-05-13 19:44:39 +02:00
|
|
|
print(f'{filename}: failed {" ".join(failed)}')
|
2015-10-19 14:53:56 +02:00
|
|
|
retval = 1
|
|
|
|
except IOError:
|
2023-05-13 19:44:39 +02:00
|
|
|
print(f'{filename}: cannot open')
|
2015-10-19 14:53:56 +02:00
|
|
|
retval = 1
|
2017-08-28 22:53:34 +02:00
|
|
|
sys.exit(retval)
|
2015-10-19 14:53:56 +02:00
|
|
|
|