Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

scripts: add MACHO PIE check to security-check.py #17787

Merged
merged 2 commits into from Jan 2, 2020

Conversation

@fanquake
Copy link
Member

fanquake commented Dec 21, 2019

This uses otool -vh to print the mach header and look for the PIE flag:

otool -vh src/bitcoind
Mach header
      magic cputype cpusubtype  caps    filetype ncmds sizeofcmds      flags
MH_MAGIC_64  X86_64        ALL LIB64     EXECUTE    24       2544   NOUNDEFS DYLDLINK TWOLEVEL WEAK_DEFINES BINDS_TO_WEAK PIE

From mach-o/loader.h:

#define	MH_PIE 0x200000			/* When this bit is set, the OS will
					   load the main executable at a
					   random address.  Only used in
					   MH_EXECUTE filetypes. */
@fanquake fanquake mentioned this pull request Dec 21, 2019
3 of 9 tasks complete
@fanquake fanquake force-pushed the fanquake:add_macOS_to_security_check branch from a4ec782 to f99430f Dec 21, 2019
@DrahtBot

This comment has been minimized.

Copy link
Contributor

DrahtBot commented Dec 22, 2019

Gitian builds

File commit 0cda557
(master)
commit f410d10
(master and this pull)
bitcoin-0.19.99-aarch64-linux-gnu-debug.tar.gz d45b6f900ad661ea... 93a8059e2cdffdb3...
bitcoin-0.19.99-aarch64-linux-gnu.tar.gz 0d5b111c83d134bf... 9167487fff9ad7cc...
bitcoin-0.19.99-arm-linux-gnueabihf-debug.tar.gz 0f12415dcf859e8e... e75a9d75a26e452d...
bitcoin-0.19.99-arm-linux-gnueabihf.tar.gz 0dce441a1e6df41a... c5c8189b9c717c06...
bitcoin-0.19.99-i686-pc-linux-gnu-debug.tar.gz c4e07651af3cb61d... 98ae87f16c95d144...
bitcoin-0.19.99-i686-pc-linux-gnu.tar.gz 087cf1d2ae102486... 42921990c000486d...
bitcoin-0.19.99-osx-unsigned.dmg b583812828864f7d... 2295dfed050febcb...
bitcoin-0.19.99-osx64.tar.gz 83118c41a5e8ab3a... 8d6c99c7edbfb41e...
bitcoin-0.19.99-riscv64-linux-gnu-debug.tar.gz 646e482bdd697f0e... 98c7d4e40a60d91f...
bitcoin-0.19.99-riscv64-linux-gnu.tar.gz cbb7b0cda942c159... 4a97a2c7253d56f7...
bitcoin-0.19.99-win64-debug.zip 5bf356f7bad511aa... f53f54cfdaeeccfe...
bitcoin-0.19.99-win64-setup-unsigned.exe 97cacb675d3fb6e4... 370dc4026a7ccc57...
bitcoin-0.19.99-win64.zip 3f90fa82138f88a0... b7ee9164965b83f7...
bitcoin-0.19.99-x86_64-linux-gnu-debug.tar.gz 7e12d1179ce41526... 84d615c7d0517be7...
bitcoin-0.19.99-x86_64-linux-gnu.tar.gz 201eae397f68469c... 986fedb9008c669f...
bitcoin-0.19.99.tar.gz ce92e2adf12e9a24... de9b2359a99ad168...
bitcoin-core-linux-0.20-res.yml 2c0f16463c33dd1d... 738a63fe32b3818a...
bitcoin-core-osx-0.20-res.yml 8d48bdfbe38d49f8... 21dca85dfab42b96...
bitcoin-core-win-0.20-res.yml 2036e58d9fe61d32... c187ce1bed90bd44...
linux-build.log a774272288af61e5... f7404ecce0c8e5c3...
osx-build.log d4429e97d2b77379... c8f635e8973590fd...
win-build.log 340a0fa5f46e0f2e... 993803432e73b05c...
bitcoin-core-linux-0.20-res.yml.diff 3daa105a128c2617...
bitcoin-core-osx-0.20-res.yml.diff 808a87d7dcd4cadd...
bitcoin-core-win-0.20-res.yml.diff 77f34938030d086e...
linux-build.log.diff 204824b486754bbf...
osx-build.log.diff d0de63551862bd45...
win-build.log.diff 02ffe7cdb7183fd2...
@bitcoin bitcoin deleted a comment from Kaylee-Cole Dec 22, 2019
@fanquake fanquake force-pushed the fanquake:add_macOS_to_security_check branch from f99430f to abf45bd Jan 2, 2020
@fanquake

This comment has been minimized.

Copy link
Member Author

fanquake commented Jan 2, 2020

Fixed up some documentation, added a function to retrieve all MACH-O flags and an additional commit to check for NOUNDEFS.

fanquake added 2 commits Jan 2, 2020
@fanquake fanquake force-pushed the fanquake:add_macOS_to_security_check branch from abf45bd to 7c9e821 Jan 2, 2020
@laanwj

This comment has been minimized.

Copy link
Member

laanwj commented Jan 2, 2020

code review ACK 7c9e821

laanwj added a commit that referenced this pull request Jan 2, 2020
7c9e821 scripts: add MACHO NOUNDEFS check to security-check.py (fanquake)
4ca92dc scripts: add MACHO PIE check to security-check.py (fanquake)

Pull request description:

  This uses `otool -vh` to print the mach header and look for the `PIE` flag:
  ```bash
  otool -vh src/bitcoind
  Mach header
        magic cputype cpusubtype  caps    filetype ncmds sizeofcmds      flags
  MH_MAGIC_64  X86_64        ALL LIB64     EXECUTE    24       2544   NOUNDEFS DYLDLINK TWOLEVEL WEAK_DEFINES BINDS_TO_WEAK PIE
  ```

  From [`mach-o/loader.h`](https://opensource.apple.com/source/cctools/cctools-927.0.2/include/mach-o/loader.h.auto.html):
  ```c
  #define	MH_PIE 0x200000			/* When this bit is set, the OS will
  					   load the main executable at a
  					   random address.  Only used in
  					   MH_EXECUTE filetypes. */
  ```

ACKs for top commit:
  laanwj:
    code review ACK 7c9e821

Tree-SHA512: 5ba2f60440d0e31c70371a355c91ca4f723d80f7287d04e2098bf5b11892cc74216ff8f1454603c4db9675d4f7983614843b992b8dcfca0309aadf2aa7ab2e4b
@laanwj laanwj merged commit 7c9e821 into bitcoin:master Jan 2, 2020
1 of 2 checks passed
1 of 2 checks passed
continuous-integration/travis-ci/pr The Travis CI build failed
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
@fanquake fanquake deleted the fanquake:add_macOS_to_security_check branch Jan 2, 2020
@MarcoFalke

This comment has been minimized.

Copy link
Member

MarcoFalke commented Jan 2, 2020

(Ir)relevant, but related discussion from IRC:

[00:06] <wumpus> happy 2020 everyone
[00:12] <fanquake> wumpus 🎉
[00:12] <wumpus> lets do my first 'git pull' of the repository this decade
[00:15] <fanquake> Watch out for all the copyright headers 🙄
[00:16] <wumpus> that  causes make / ccache to forget everything resulting in a clean build for the new year!
[00:20] <fanquake> heh. Looking forward to another decade of bitcoind!
[00:21] <wumpus> yes!
[00:22] <wumpus> what would be cool to merge as first PR?
[00:23] <wumpus> #10785 would be nice
[00:23] <gribble> https://github.com/bitcoin/bitcoin/issues/10785 | Serialization improvements by sipa · Pull Request #10785 · bitcoin/bitcoin · GitHub
[00:24] <fanquake> Right now I can probably only tell you what would not be cool to merge
[00:24] <wumpus> very futuristic to have improved serialization for the new decade
[00:24] <wumpus> hehe yes
[00:26] <sipa> agree! ;)
[00:27] <fanquake> I'll be very biased, an suggest that #17787 is just a tiny bit "cool"
[00:27] <gribble> https://github.com/bitcoin/bitcoin/issues/17787 | scripts: add MACHO PIE check to security-check.py by fanquake · Pull Request #17787 · bitcoin/bitcoin · GitHub
[00:28] <sipa> MACHO does sound very cool
[00:30] <fanquake> Got some pie as well. Sounds like a good deal all round
[00:31] <wumpus> MACHO PIE
[00:32] <wumpus> it's good branding for security features
[00:35] <fanquake> macOS has all the cool flags heh
[00:35] <fanquake> NOUNDEFS DYLDLINK TWOLEVEL WEAK_DEFINES BINDS_TO_WEAK PIE
sidhujag added a commit to syscoin/syscoin that referenced this pull request Jan 2, 2020
7c9e821 scripts: add MACHO NOUNDEFS check to security-check.py (fanquake)
4ca92dc scripts: add MACHO PIE check to security-check.py (fanquake)

Pull request description:

  This uses `otool -vh` to print the mach header and look for the `PIE` flag:
  ```bash
  otool -vh src/bitcoind
  Mach header
        magic cputype cpusubtype  caps    filetype ncmds sizeofcmds      flags
  MH_MAGIC_64  X86_64        ALL LIB64     EXECUTE    24       2544   NOUNDEFS DYLDLINK TWOLEVEL WEAK_DEFINES BINDS_TO_WEAK PIE
  ```

  From [`mach-o/loader.h`](https://opensource.apple.com/source/cctools/cctools-927.0.2/include/mach-o/loader.h.auto.html):
  ```c
  #define	MH_PIE 0x200000			/* When this bit is set, the OS will
  					   load the main executable at a
  					   random address.  Only used in
  					   MH_EXECUTE filetypes. */
  ```

ACKs for top commit:
  laanwj:
    code review ACK 7c9e821

Tree-SHA512: 5ba2f60440d0e31c70371a355c91ca4f723d80f7287d04e2098bf5b11892cc74216ff8f1454603c4db9675d4f7983614843b992b8dcfca0309aadf2aa7ab2e4b
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.