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

2 files changed, 26 insertions, 7 deletions

diff --git a/contrib/devtools/security-check.py b/contrib/devtools/security-check.py
index b924698e56..65a80b4102 100755
--- a/contrib/devtools/security-check.py
+++ b/contrib/devtools/security-check.py
@@ -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)
 ]
 }
 
diff --git a/contrib/devtools/test-security-check.py b/contrib/devtools/test-security-check.py
index db738fbbb8..d09f1d0064 100755
--- a/contrib/devtools/test-security-check.py
+++ b/contrib/devtools/test-security-check.py
@@ -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__':