Skip to content

Conversation

fanquake
Copy link
Member

ASLR is not currently working for the bitcoin-cli.exe binary. This is
due to it not having a .reloc section, which is stripped by default by
the mingw-w64 ld we use for gitian builds. A good summary of issues with
ld and mingw-w64 is available in this thread:
https://sourceware.org/bugzilla/show_bug.cgi?id=19011.

All other Windows binaries that we distribute (bitcoind, bitcoin-qt,
bitcoin-wallet, bitcoin-tx and test_bitcoin) do not suffer this issue,
and currently having working ASLR. This is due to them exporting
(inadvertent or not) libsecp256k1 symbols, and, as a result, the .reloc
section is not stripped by ld.

This change is a temporary workaround, also the same one described here:
https://www.kb.cert.org/vuls/id/307144/, that causes main() to be
exported. Exporting a symbol will mean that the .reloc section is not
stripped, and ASLR will function correctly.

Ultimately, this will be fixed by using a newer version of binutils (that has this change). Whether that's through bumping our gitian distro, or Guix.

Related to #18629, which has a bunch of additional information in the PR description. If you would like to verify whether or not ASLR is indeed working, with or without this change. One easy way to check is using a tool like VMMap.

Here are the memory mappings for the 0.20.0rc1 bitcoind.exe and bitcoin-cli.exe binaries. You'll notice that over machine restarts, even though the image is marked (ASLR) (which I assume may be due to the header bit being set), no ASLR is actually occuring for bitcoin-cli.exe:

bitcoind.exe

bitcoind-1

bitcoind-2

bitcoind-3

bitcoin-cli.exe

bitcoin-cli-1

bitcoin-cli-2

bitcoin-cli-3

ASLR is not currently working for the bitcoin-cli.exe binary. This is
due to it not having a .reloc section, which is stripped by default by
the mingw-w64 ld we use for gitian builds. A good summary of issues with
ld and mingw-w64 is available in this thread:
https://sourceware.org/bugzilla/show_bug.cgi?id=19011.

All other Windows binaries that we distribute (bitcoind, bitcoin-qt,
bitcoin-wallet, bitcoin-tx and test_bitcoin) do not suffer this issue,
and currently having working ASLR. This is due to them exporting
(inadvertent or not) libsecp256k1 symbols, and, as a result, the .reloc
section is not stripped by ld.

This change is a temporary workaround, also the same one described here:
https://www.kb.cert.org/vuls/id/307144/, that causes main() to be
exported. Exporting a symbol will mean that the .reloc section is not
stripped, and ASLR will function correctly.
@DrahtBot
Copy link
Contributor

Gitian builds

File commit 6ae99aa
(master)
commit d237dea
(master and this pull)
bitcoin-0.20.99-aarch64-linux-gnu-debug.tar.gz 9e74baea5087f75d... 17b7170b00c0dd88...
bitcoin-0.20.99-aarch64-linux-gnu.tar.gz b97f8fed4ef242bc... da9a6944b9be6b4c...
bitcoin-0.20.99-arm-linux-gnueabihf-debug.tar.gz c3cafca0d2c385b7... e6676a2849cfa71b...
bitcoin-0.20.99-arm-linux-gnueabihf.tar.gz 9cf7842f6c7e72f9... d71bb140861e5eca...
bitcoin-0.20.99-osx-unsigned.dmg b8882f2b1e257bee... 52df1d0523647495...
bitcoin-0.20.99-osx64.tar.gz 9100c2ace54ffeb2... 4985eb0096483c4c...
bitcoin-0.20.99-riscv64-linux-gnu-debug.tar.gz e3794553bdeb3406... 0345a24d96cca6a2...
bitcoin-0.20.99-riscv64-linux-gnu.tar.gz 3d60c7b6ad3a3b7a... 0783727f20114c82...
bitcoin-0.20.99-win64-debug.zip 923a2a8d708b1196... 8d339f57d6fe94e3...
bitcoin-0.20.99-win64-setup-unsigned.exe 4fd60fb4ee6d252a... 55d2abd0b43819dc...
bitcoin-0.20.99-win64.zip 014a0e245da57de1... f22edf0c92378827...
bitcoin-0.20.99-x86_64-linux-gnu-debug.tar.gz 6b72060bfdf9cd8a... e289c83cd70e9e33...
bitcoin-0.20.99-x86_64-linux-gnu.tar.gz d2e0d4726ecd84f6... 0a21c16ad59f670b...
bitcoin-0.20.99.tar.gz 5f8eaab85a59877b... 5b40dd5eeb98dabb...
bitcoin-core-linux-0.21-res.yml b9c0b8516f1f7476... 3002aa03ee950e1b...
bitcoin-core-osx-0.21-res.yml aa56d3073ba6853c... e64d1a4f3ec80411...
bitcoin-core-win-0.21-res.yml 127d7553b4c8553c... fa50baed0f18ce1f...
linux-build.log 826d7ba2297e1251... 12a723ef0886c1f0...
osx-build.log a889d7e96df8cc6a... 18583e3a9431da5f...
win-build.log 6311c7e3c77d0b67... 33da40887ebdda26...
bitcoin-core-linux-0.21-res.yml.diff e14541ca07136f09...
bitcoin-core-osx-0.21-res.yml.diff 5cbb2d4ee9620e1b...
bitcoin-core-win-0.21-res.yml.diff a10fe6ab80d289a3...
linux-build.log.diff 758faaa56e5468ec...
osx-build.log.diff 66114758316db2da...
win-build.log.diff a4119d365f724743...

@dongcarl
Copy link
Contributor

ACK 315a4d3
Travis failure seems to be because of #18691

@laanwj
Copy link
Member

laanwj commented Apr 22, 2020

Thanks for the clear description of the issue in the OP and comment.

ACK 315a4d3

(restarted failed travis run)

@laanwj laanwj merged commit 5dcb061 into bitcoin:master Apr 22, 2020
@fanquake fanquake deleted the fix_pe_cli_aslr branch April 23, 2020 00:42
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Apr 23, 2020
315a4d3 build: fix ASLR for bitcoin-cli on Windows (fanquake)

Pull request description:

  ASLR is not currently working for the `bitcoin-cli.exe` binary. This is
  due to it not having a .reloc section, which is stripped by default by
  the mingw-w64 ld we use for gitian builds. A good summary of issues with
  ld and mingw-w64 is available in this thread:
  https://sourceware.org/bugzilla/show_bug.cgi?id=19011.

  All other Windows binaries that we distribute (bitcoind, bitcoin-qt,
  bitcoin-wallet, bitcoin-tx and test_bitcoin) do not suffer this issue,
  and currently having working ASLR. This is due to them exporting
  (inadvertent or not) libsecp256k1 symbols, and, as a result, the .reloc
  section is not stripped by ld.

  This change is a temporary workaround, also the same one described here:
  https://www.kb.cert.org/vuls/id/307144/, that causes main() to be
  exported. Exporting a symbol will mean that the .reloc section is not
  stripped, and ASLR will function correctly.

  Ultimately, this will be fixed by using a newer version of binutils (that has this [change](https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;a=commit;h=dc9bd8c92af67947db44b3cb428c050259b15cd0)). Whether that's through bumping our gitian distro, or Guix.

  Related to bitcoin#18629, which has a bunch of additional information in the PR description. If you would like to verify whether or not ASLR is indeed working, with or without this change. One easy way to check is using a tool like [VMMap](https://docs.microsoft.com/en-us/sysinternals/downloads/vmmap).

  Here are the memory mappings for the 0.20.0rc1 `bitcoind.exe` and `bitcoin-cli.exe` binaries. You'll notice that over machine restarts, even though the image is marked `(ASLR)` (which I assume may be due to the header bit being set), no ASLR is actually occuring for `bitcoin-cli.exe`:

  #### bitcoind.exe

  ![bitcoind-1](https://user-images.githubusercontent.com/863730/79678203-74065c80-822b-11ea-90bc-9c883d0aeefa.png)

  ![bitcoind-2](https://user-images.githubusercontent.com/863730/79678204-7668b680-822b-11ea-9263-3e7ba22f904c.png)

  ![bitcoind-3](https://user-images.githubusercontent.com/863730/79678206-7963a700-822b-11ea-972f-af31a514b9b4.png)

  #### bitcoin-cli.exe

  ![bitcoin-cli-1](https://user-images.githubusercontent.com/863730/79678208-7ec0f180-822b-11ea-8480-a4b5d1762945.png)

  ![bitcoin-cli-2](https://user-images.githubusercontent.com/863730/79678213-81bbe200-822b-11ea-964d-994f58ff12b0.png)

  ![bitcoin-cli-3](https://user-images.githubusercontent.com/863730/79678215-84b6d280-822b-11ea-9cd6-fee2e239c003.png)

ACKs for top commit:
  dongcarl:
    ACK 315a4d3
  laanwj:
    ACK 315a4d3

Tree-SHA512: 95f4dc15420ed9bcdeacb763e11c3c7e563eec594a172746fa0346c13f97db3a8769357dffc89fea1e57ae67133f337b1013a73b584662f5b6c4d251ca20a2b1
fanquake added a commit to fanquake/bitcoin that referenced this pull request Apr 23, 2020
ASLR is not currently working for the bitcoin-cli.exe binary. This is
due to it not having a .reloc section, which is stripped by default by
the mingw-w64 ld we use for gitian builds. A good summary of issues with
ld and mingw-w64 is available in this thread:
https://sourceware.org/bugzilla/show_bug.cgi?id=19011.

All other Windows binaries that we distribute (bitcoind, bitcoin-qt,
bitcoin-wallet, bitcoin-tx and test_bitcoin) do not suffer this issue,
and currently having working ASLR. This is due to them exporting
(inadvertent or not) libsecp256k1 symbols, and, as a result, the .reloc
section is not stripped by ld.

This change is a temporary workaround, also the same one described here:
https://www.kb.cert.org/vuls/id/307144/, that causes main() to be
exported. Exporting a symbol will mean that the .reloc section is not
stripped, and ASLR will function correctly.

Github-Pull: bitcoin#18702
Rebased-From: 315a4d3
@fanquake fanquake mentioned this pull request Apr 23, 2020
laanwj added a commit that referenced this pull request May 11, 2020
7f7548d rpc: Do not advertise dumptxoutset as a way to flush the chainstate (MarcoFalke)
a9ca65b Fix naming of macOS SDK and clarify version (Andrew Chow)
54d2063 Do not expose and consider -logthreadnames when it does not work (Hennadii Stepanov)
6986b26 build: fix ASLR for bitcoin-cli on Windows (fanquake)
1d1e358 build: Set libevent minimum version to 2.0.21 (Hennadii Stepanov)
842b13a Avoid non-trivial global constants in SHA-NI code (Pieter Wuille)
ade4185 gitian: Add missing automake package to gitian-win-signer.yml (Andrew Chow)

Pull request description:

  Currently backports the following to the 0.20 branch:

  * #18598 - gitian: Add missing automake package to gitian-win-signer.yml
  * #18702 - build: fix ASLR for bitcoin-cli on Windows
  * #18676 - build: Check libevent minimum version in configure script
  * #18665 - Do not expose and consider -logthreadnames when it does not work
  * #18553 - Avoid non-trivial global constants in SHA-NI code
  * #18589 - Fix naming of macOS SDK and clarify version

ACKs for top commit:
  laanwj:
    ACK 7f7548d

Tree-SHA512: 2cba748414a440e3fb901940085a7ae059e8b926c9187fbbbdeb7846a32e7374f318cc21e499c911ff803c42aef2c844b04af10b87f9c5a2b3edf6deb03ebb04
fanquake added a commit to fanquake/bitcoin that referenced this pull request Jan 15, 2021
The binutils we use for gitian builds strips the reloc section from
Windows binaries, which breaks ASLR. As a temporary workaround, export
main(). This is the same workaround as bitcoin#18702 (bitcoin-cli), and will
fix the currently failing security check:
```bash
+ make -j1 -C src check-security
make: Entering directory '/home/ubuntu/build/bitcoin/distsrc-x86_64-w64-mingw32/src'
Checking binary security...
bitcoin-util.exe: failed RELOC_SECTION
make: *** [check-security] Error 1
```

Relevant upstream issue:
https://sourceware.org/bugzilla/show_bug.cgi?id=19011
fanquake added a commit to fanquake/bitcoin that referenced this pull request Jan 15, 2021
The binutils we use for gitian builds strips the reloc section from
Windows binaries, which breaks ASLR. As a temporary workaround, export
main(). This is the same workaround as bitcoin#18702 (bitcoin-cli), and will
fix the currently failing security check:
```bash
+ make -j1 -C src check-security
make: Entering directory '/home/ubuntu/build/bitcoin/distsrc-x86_64-w64-mingw32/src'
Checking binary security...
bitcoin-util.exe: failed RELOC_SECTION
make: *** [check-security] Error 1
```

Relevant upstream issue:
https://sourceware.org/bugzilla/show_bug.cgi?id=19011
laanwj added a commit that referenced this pull request Jan 17, 2021
c061800 build: fix RELOC_SECTION security check for bitcoin-util (fanquake)

Pull request description:

  The binutils we use for gitian builds strips the reloc section from
  Windows binaries, which breaks ASLR. As a temporary workaround, export
  main(). This is the same workaround as #18702 (bitcoin-cli), and will
  fix the currently failing security check:
  ```bash
  + make -j1 -C src check-security
  make: Entering directory '/home/ubuntu/build/bitcoin/distsrc-x86_64-w64-mingw32/src'
  Checking binary security...
  bitcoin-util.exe: failed RELOC_SECTION
  make: *** [check-security] Error 1
  ```

  Relevant upstream issue:
  https://sourceware.org/bugzilla/show_bug.cgi?id=19011

ACKs for top commit:
  dongcarl:
    ACK c061800
  laanwj:
    ACK c061800

Tree-SHA512: a1a4da0b2cddfc377190b9044a04f42a859ca79f11ce2c2ab4c3d066a2786c34d5446d75f8eec634f308d2d3349ebbd7c9f76dcaebeeb28e471c829851592367
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Jan 17, 2021
…oin-util

c061800 build: fix RELOC_SECTION security check for bitcoin-util (fanquake)

Pull request description:

  The binutils we use for gitian builds strips the reloc section from
  Windows binaries, which breaks ASLR. As a temporary workaround, export
  main(). This is the same workaround as bitcoin#18702 (bitcoin-cli), and will
  fix the currently failing security check:
  ```bash
  + make -j1 -C src check-security
  make: Entering directory '/home/ubuntu/build/bitcoin/distsrc-x86_64-w64-mingw32/src'
  Checking binary security...
  bitcoin-util.exe: failed RELOC_SECTION
  make: *** [check-security] Error 1
  ```

  Relevant upstream issue:
  https://sourceware.org/bugzilla/show_bug.cgi?id=19011

ACKs for top commit:
  dongcarl:
    ACK c061800
  laanwj:
    ACK c061800

Tree-SHA512: a1a4da0b2cddfc377190b9044a04f42a859ca79f11ce2c2ab4c3d066a2786c34d5446d75f8eec634f308d2d3349ebbd7c9f76dcaebeeb28e471c829851592367
remyers pushed a commit to remyers/bitcoin that referenced this pull request Jan 26, 2021
The binutils we use for gitian builds strips the reloc section from
Windows binaries, which breaks ASLR. As a temporary workaround, export
main(). This is the same workaround as bitcoin#18702 (bitcoin-cli), and will
fix the currently failing security check:
```bash
+ make -j1 -C src check-security
make: Entering directory '/home/ubuntu/build/bitcoin/distsrc-x86_64-w64-mingw32/src'
Checking binary security...
bitcoin-util.exe: failed RELOC_SECTION
make: *** [check-security] Error 1
```

Relevant upstream issue:
https://sourceware.org/bugzilla/show_bug.cgi?id=19011
backpacker69 referenced this pull request in peercoin/peercoin Mar 28, 2021
ASLR is not currently working for the bitcoin-cli.exe binary. This is
due to it not having a .reloc section, which is stripped by default by
the mingw-w64 ld we use for gitian builds. A good summary of issues with
ld and mingw-w64 is available in this thread:
https://sourceware.org/bugzilla/show_bug.cgi?id=19011.

All other Windows binaries that we distribute (bitcoind, bitcoin-qt,
bitcoin-wallet, bitcoin-tx and test_bitcoin) do not suffer this issue,
and currently having working ASLR. This is due to them exporting
(inadvertent or not) libsecp256k1 symbols, and, as a result, the .reloc
section is not stripped by ld.

This change is a temporary workaround, also the same one described here:
https://www.kb.cert.org/vuls/id/307144/, that causes main() to be
exported. Exporting a symbol will mean that the .reloc section is not
stripped, and ASLR will function correctly.

Github-Pull: #18702
Rebased-From: 315a4d3
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Feb 15, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants