Merge #18713: scripts: Add MACHO stack canary check to security-check.py

8334ee31f868f0f9baf0920d14d20174ed889dbe scripts: add MACHO LAZY_BINDINGS test to test-security-check.py (fanquake)
7b99c7454cdb74cd9cd7a5eedc2fb9d0a19df456 scripts: add MACHO Canary check to security-check.py (fanquake)

Pull request description:

  7b99c7454cdb74cd9cd7a5eedc2fb9d0a19df456 uses `otool -Iv` to check for `___stack_chk_fail` in the macOS binaries. Similar to the [ELF check](https://github.com/bitcoin/bitcoin/blob/master/contrib/devtools/security-check.py#L105). Note that looking for a triple underscore prefixed function (as opposed to two for ELF) is correct for the macOS binaries. i.e:
  ```bash
  otool -Iv bitcoind | grep chk
  0x00000001006715b8   509 ___memcpy_chk
  0x00000001006715be   510 ___snprintf_chk
  0x00000001006715c4   511 ___sprintf_chk
  0x00000001006715ca   512 ___stack_chk_fail
  0x00000001006715d6   517 ___vsnprintf_chk
  0x0000000100787898   513 ___stack_chk_guard
  ```

  8334ee31f868f0f9baf0920d14d20174ed889dbe is a follow up to #18295 and adds test cases to `test-security-check.py` that for some reason I didn't add at the time. I'll sort out #18434 so that we can run these tests in the CI.

ACKs for top commit:
  practicalswift:
    ACK 8334ee31f868f0f9baf0920d14d20174ed889dbe: Mitigations are important. Important things are worth asserting :)
  jonasschnelli:
    utACK 8334ee31f868f0f9baf0920d14d20174ed889dbe.

Tree-SHA512: 1aa5ded34bbd187eddb112b27278deb328bfc21ac82316b20fab6ad894f223b239a76b53dab0ac1770d194c1760fcc40d4da91ec09959ba4fc8eadedb173936a
This commit is contained in:
fanquake 2020-04-22 16:19:51 +08:00 committed by pasta
parent 96866c4579
commit bd7b5a3ad4
No known key found for this signature in database
GPG Key ID: 52527BEDABE87984
2 changed files with 26 additions and 7 deletions

View File

@ -223,6 +223,20 @@ def check_MACHO_LAZY_BINDINGS(executable) -> bool:
return False
return True
def check_MACHO_Canary(executable) -> bool:
'''
Check for use of stack canary
'''
p = subprocess.Popen([OTOOL_CMD, '-Iv', executable], stdout=subprocess.PIPE, stderr=subprocess.PIPE, stdin=subprocess.PIPE, universal_newlines=True)
(stdout, stderr) = p.communicate()
if p.returncode:
raise IOError('Error opening file')
ok = False
for line in stdout.splitlines():
if '___stack_chk_fail' in line:
ok = True
return ok
CHECKS = {
'ELF': [
('PIE', check_ELF_PIE),
@ -239,7 +253,8 @@ CHECKS = {
('PIE', check_MACHO_PIE),
('NOUNDEFS', check_MACHO_NOUNDEFS),
('NX', check_MACHO_NX),
('LAZY_BINDINGS', check_MACHO_LAZY_BINDINGS)
('LAZY_BINDINGS', check_MACHO_LAZY_BINDINGS),
('Canary', check_MACHO_Canary)
]
}

View File

@ -64,13 +64,17 @@ class TestSecurityChecks(unittest.TestCase):
cc = 'clang'
write_testcode(source)
self.assertEqual(call_security_check(cc, source, executable, ['-Wl,-no_pie','-Wl,-flat_namespace', '-Wl,-allow_stack_execute']),
(1, executable+': failed PIE NOUNDEFS NX'))
self.assertEqual(call_security_check(cc, source, executable, ['-Wl,-no_pie','-Wl,-flat_namespace']),
(1, executable+': failed PIE NOUNDEFS'))
self.assertEqual(call_security_check(cc, source, executable, ['-Wl,-no_pie']),
self.assertEqual(call_security_check(cc, source, executable, ['-Wl,-no_pie','-Wl,-flat_namespace','-Wl,-allow_stack_execute','-fno-stack-protector']),
(1, executable+': failed PIE NOUNDEFS NX LAZY_BINDINGS Canary'))
self.assertEqual(call_security_check(cc, source, executable, ['-Wl,-no_pie','-Wl,-flat_namespace','-Wl,-allow_stack_execute','-fstack-protector-all']),
(1, executable+': failed PIE NOUNDEFS NX LAZY_BINDINGS'))
self.assertEqual(call_security_check(cc, source, executable, ['-Wl,-no_pie','-Wl,-flat_namespace','-fstack-protector-all']),
(1, executable+': failed PIE NOUNDEFS LAZY_BINDINGS'))
self.assertEqual(call_security_check(cc, source, executable, ['-Wl,-no_pie','-fstack-protector-all']),
(1, executable+': failed PIE LAZY_BINDINGS'))
self.assertEqual(call_security_check(cc, source, executable, ['-Wl,-no_pie','-Wl,-bind_at_load','-fstack-protector-all']),
(1, executable+': failed PIE'))
self.assertEqual(call_security_check(cc, source, executable, ['-Wl,-pie']),
self.assertEqual(call_security_check(cc, source, executable, ['-Wl,-pie','-Wl,-bind_at_load','-fstack-protector-all']),
(0, ''))
if __name__ == '__main__':