GCC hardening for Bitcoin-Qt #1925

pull Diapolo wants to merge 2 commits into bitcoin:master from Diapolo:gcc-hardening changing 1 files +4 −2

Diapolo commented at 9:28 PM on October 12, 2012: none
- Bitcoin-Qt: add new GCC compiler hardening options
- Bitcoin-Qt: remove unneeded "--param ssp-buffer-size=1" flag

Bitcoin-Qt: add new GCC compiler hardening options

- this patch enables several new GCC compiler hardening options that
  allows us to increase the security of our binaries (see:
  https://wiki.debian.org/Hardening)

-D_FORTIFY_SOURCE=2:
Enables compile-time protection against static sized buffer overflows.

-Wl,-z,relro -Wl,-z,now:
Enables full RELRO (RELocation Read-Only), which is a generic mitigation
technique to harden the data sections of an ELF binary/process. See:
http://isisblogs.poly.edu/2011/06/01/relro-relocation-read-only/ for
further details.

10d3603ffa

Bitcoin-Qt: remove unneeded "--param ssp-buffer-size=1" flag

- that flag is not needed when using "-fstack-protector-all", so remove it
  (see:
  http://stackoverflow.com/questions/1629685/when-and-how-to-use-gccs-stack-protection-feature)

493940038f

in src/makefile.unix:None in cef8a7a8dc outdated
```
  67 | @@ -68,7 +68,7 @@ LIBS+= \
  68 |      # If a stack buffer if overflowed, it writes over the canary number and then on return
  69 |      # when that number is checked, it won't be the same and the program will exit with
  70 |      # a "Stack smashing detected" error instead of being exploited.
  71 | -    HARDENING+=-fstack-protector-all -Wstack-protector
  72 | +    HARDENING+=-fstack-protector-all --param ssp-buffer-size=1 -Wstack-protector
```
gavinandresen commented at 9:48 PM on October 12, 2012:

I thought -fstack_protector-all makes the ssp-buffer-size param irrelevant; am I wrong?

Diapolo commented at 9:51 PM on October 12, 2012:

I'm not sure as the GCC documentation is quiet about that: http://gcc.gnu.org/onlinedocs/gcc-4.7.1/gcc/Optimize-Options.html#index-fstack_002dprotector_002dall-901

And we use it for Bitcoin-Qt, too. Do you have a source to verify --param ssp-buffer-size=1 is set when -fstack-protector-all is used?

gavinandresen commented at 10:35 PM on October 12, 2012:

ssp-buffer-size is irrelevant with -all, because ALL functions are protected.

See http://stackoverflow.com/questions/1629685/when-and-how-to-use-gccs-stack-protection-feature "...at a size of 0/1, it is the same as stack-protector-all."

laanwj commented at 7:12 AM on October 13, 2012:

Cool, let's also remove it from bitcoin-qt.pro, then

Diapolo commented at 8:26 AM on October 13, 2012:

I'll update this pull :), thanks Gavin.

in bitcoin-qt.pro:None in cef8a7a8dc outdated

  37 | @@ -38,6 +38,8 @@ QMAKE_LFLAGS *= -fstack-protector-all --param ssp-buffer-size=1
  38 |  # We need to exclude this for Windows cross compile with MinGW 4.2.x, as it will result in a non-working executable!
  39 |  # This can be enabled for Windows, when we switch to MinGW >= 4.4.x.
  40 |  }
  41 | +# for extra security (see: https://wiki.debian.org/Hardening)
  42 | +QMAKE_CXXFLAGS *= -D_FORTIFY_SOURCE=2 -Wl,-z,relro -Wl,-z,now

laanwj commented at 7:12 AM on October 13, 2012:

ACK

Diapolo commented at 8:29 AM on October 13, 2012: none

Updated to remove the "--param ssp-buffer-size=1" flag from the project file, no changes to makefile.unix.
BitcoinPullTester commented at 8:48 PM on October 19, 2012: none

Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/493940038f44412ece135e1a625499ea89a93e56 for binaries and test log.
Diapolo commented at 8:06 AM on October 25, 2012: none

@laanwj Any reason to not merge this now?
laanwj referenced this in commit 2d53ba2c69 on Oct 25, 2012
laanwj merged this on Oct 25, 2012
laanwj closed this on Oct 25, 2012
laudney referenced this in commit a61fdcf146 on Mar 19, 2014
DrahtBot locked this on Sep 8, 2021

Contributors