build: Add Stack-based Buffer Overrun Detection (-fstack-protector)

Version: All
Platform: Platforms that support this flag
Subsystem:

This issue is created to track the Stack-based Buffer Overrun Detection (-fstack-protector) suggestion from issue #18671 which included several different build flags.

The basic idea of Stack-based Buffer Overrun Detection building flag is to add a "canary" which is a
random number after the return address in the stack and check its value before returns to make sure it's not altered. Thus it could avoid a lot of stack buffer overflow attacks.

This could introduce more security but also performance lost, mainly because of the check of the canary. So, 3 different modes were provided to control how many functions are covered thus providing the trade off between security and performance. They are:

-fstack-protector
-fstack-protector-all
-fstack-protector-strong.

The -fstack-protector-all mode would affect every function and is not acceptable.
The -fstack-protector protects functions that have a character array of eight bytes or more in length on its stack which seems too restrictive.
The -fstack-protector-strong mode protects more functions than -fstack-protector but not expand to all, so is a balance between -fstack-protector and -fstack-protector-all. Some info can be found here. However it's not supported in clang 3.4.2 (while it seems it's supported from clang 3.5.0 ).

Maybe in future clang baseline increased to >= 3.5.0 we could have a try on -fstack-protector-strong. Currently we could focus on -fstack-protector, and I am taking on the performance test on it to see its impact on performance and would make a PR if the result is acceptable.

The benchmark test result is as follows, I found buffers, string_decoder and url tests are impacted more or less, then I took 30 runs on them.
My machine:
CPU: Intel(R) Core(TM) i7-4790 CPU @ 3.60GHz with 4 cores
RAM: 32G
OS: Ubuntu 16.04 LTS

I picked the most significant data from those results:

For benchmark/buffers:
buffers/buffer-fill.js n=20000 size=8192 type='fill("test")'                     ***    -11.11 %       ±1.79% ±2.38% ±3.10%

For benchmark/string_decoder:
 string_decoder/string-decoder.js n=2500000 chunkLen=64 inLen=4096 encoding='utf8'        ***    -13.45 %       ±1.19% ±1.60% ±2.11%

For benchmark/url:
 url/whatwg-url-properties.js n=300000 prop='host' input='long'              ***     -7.57 %       ±1.49% ±1.98% ±2.59%

I did suggest to enable this flag, as linux kernel and chrome also enabled this. Balance security with performance are indeed a trade off. The thing I am wondering is whether this impact can be accepted? If yes I would prepare a PR for that. Thanks.

That's a pretty steep performance drop. Can you post the full results?

I didn't take a full benchmark for call cases with 30 runs, that seems too time costing. I just run all of the cases for about 3 runs, then choose the cases that have significant impacts and then I rerun those selected cases with 30 runs.

I even removed some .js files and removed some configs by edit some js files, only focusing on the cases with significant impacts. So you may notice in my result there are few text than normal.

Below is my results, if it's not enough, I could re-run more cases then. :)
result of benchmark/buffers:

                                                                           confidence improvement accuracy (*)   (**)  (***)
 buffers/buffer-fill.js n=20000 size=256 type='fill("")'                                   1.02 %       ±3.26% ±4.34% ±5.65%
 buffers/buffer-fill.js n=20000 size=256 type='fill(0)'                                   -0.88 %       ±1.60% ±2.13% ±2.77%
 buffers/buffer-fill.js n=20000 size=256 type='fill(100)'                                 -0.09 %       ±2.15% ±2.87% ±3.73%
 buffers/buffer-fill.js n=20000 size=256 type='fill(400)'                                  0.45 %       ±1.19% ±1.60% ±2.10%
 buffers/buffer-fill.js n=20000 size=256 type='fill(Buffer.alloc(1), 0)'           **      2.02 %       ±1.24% ±1.66% ±2.18%
 buffers/buffer-fill.js n=20000 size=256 type='fill("t")'                                  1.35 %       ±3.76% ±5.01% ±6.52%
 buffers/buffer-fill.js n=20000 size=256 type='fill("t", 0)'                              -1.92 %       ±3.02% ±4.02% ±5.24%
 buffers/buffer-fill.js n=20000 size=256 type='fill("t", 0, "utf8")'                       1.56 %       ±2.97% ±3.96% ±5.15%
 buffers/buffer-fill.js n=20000 size=256 type='fill("test")'                               1.03 %       ±2.49% ±3.32% ±4.32%
 buffers/buffer-fill.js n=20000 size=256 type='fill("t", "utf8")'                          2.61 %       ±3.58% ±4.77% ±6.20%
 buffers/buffer-fill.js n=20000 size=65536 type='fill("")'                                 0.23 %       ±0.46% ±0.61% ±0.79%
 buffers/buffer-fill.js n=20000 size=65536 type='fill(0)'                                  0.09 %       ±0.40% ±0.53% ±0.69%
 buffers/buffer-fill.js n=20000 size=65536 type='fill(100)'                               -0.24 %       ±0.35% ±0.47% ±0.61%
 buffers/buffer-fill.js n=20000 size=65536 type='fill(400)'                               -0.03 %       ±0.28% ±0.38% ±0.49%
 buffers/buffer-fill.js n=20000 size=65536 type='fill(Buffer.alloc(1), 0)'        ***     -5.31 %       ±0.56% ±0.74% ±0.98%
 buffers/buffer-fill.js n=20000 size=65536 type='fill("t")'                         *      0.48 %       ±0.41% ±0.54% ±0.70%
 buffers/buffer-fill.js n=20000 size=65536 type='fill("t", 0)'                             0.15 %       ±0.45% ±0.61% ±0.79%
 buffers/buffer-fill.js n=20000 size=65536 type='fill("t", 0, "utf8")'                    -0.17 %       ±0.54% ±0.71% ±0.93%
 buffers/buffer-fill.js n=20000 size=65536 type='fill("test")'                    ***      6.85 %       ±0.67% ±0.89% ±1.17%
 buffers/buffer-fill.js n=20000 size=65536 type='fill("t", "utf8")'                       -0.37 %       ±0.49% ±0.65% ±0.84%
 buffers/buffer-fill.js n=20000 size=8192 type='fill("")'                                 -1.78 %       ±2.55% ±3.39% ±4.41%
 buffers/buffer-fill.js n=20000 size=8192 type='fill(0)'                                  -0.85 %       ±1.17% ±1.56% ±2.03%
 buffers/buffer-fill.js n=20000 size=8192 type='fill(100)'                                -0.12 %       ±1.03% ±1.37% ±1.78%
 buffers/buffer-fill.js n=20000 size=8192 type='fill(400)'                                -0.73 %       ±0.97% ±1.29% ±1.68%
 buffers/buffer-fill.js n=20000 size=8192 type='fill(Buffer.alloc(1), 0)'         ***      6.60 %       ±0.96% ±1.28% ±1.66%
 buffers/buffer-fill.js n=20000 size=8192 type='fill("t")'                                 1.31 %       ±2.90% ±3.85% ±5.01%
 buffers/buffer-fill.js n=20000 size=8192 type='fill("t", 0)'                              1.17 %       ±2.95% ±3.93% ±5.13%
 buffers/buffer-fill.js n=20000 size=8192 type='fill("t", 0, "utf8")'                     -0.74 %       ±2.37% ±3.16% ±4.11%
 buffers/buffer-fill.js n=20000 size=8192 type='fill("test")'                     ***    -11.11 %       ±1.79% ±2.38% ±3.10%
 buffers/buffer-fill.js n=20000 size=8192 type='fill("t", "utf8")'                 **     -4.50 %       ±3.20% ±4.27% ±5.56%
 buffers/buffer-from.js n=2048 len=10 source='array'                              ***     -4.35 %       ±1.55% ±2.07% ±2.71%
 buffers/buffer-from.js n=2048 len=10 source='arraybuffer'                         **     -3.16 %       ±1.94% ±2.62% ±3.47%
 buffers/buffer-from.js n=2048 len=10 source='arraybuffer-middle'                 ***     -1.76 %       ±0.82% ±1.11% ±1.46%
 buffers/buffer-from.js n=2048 len=10 source='buffer'                              **      0.68 %       ±0.42% ±0.56% ±0.74%
 buffers/buffer-from.js n=2048 len=10 source='object'                             ***      7.83 %       ±3.63% ±4.89% ±6.48%
 buffers/buffer-from.js n=2048 len=10 source='string'                                     -1.48 %       ±1.76% ±2.37% ±3.15%
 buffers/buffer-from.js n=2048 len=10 source='string-base64'                              -0.38 %       ±1.41% ±1.89% ±2.48%
 buffers/buffer-from.js n=2048 len=10 source='string-utf8'                          *     -2.06 %       ±1.75% ±2.33% ±3.03%
 buffers/buffer-from.js n=2048 len=10 source='uint8array'                                  1.09 %       ±1.64% ±2.18% ±2.84%
 buffers/buffer-from.js n=2048 len=2048 source='array'                                     0.01 %       ±0.09% ±0.12% ±0.15%
 buffers/buffer-from.js n=2048 len=2048 source='arraybuffer'                      ***     -2.14 %       ±0.37% ±0.49% ±0.64%
 buffers/buffer-from.js n=2048 len=2048 source='arraybuffer-middle'                       -0.29 %       ±1.81% ±2.44% ±3.23%
 buffers/buffer-from.js n=2048 len=2048 source='buffer'                                   -0.02 %       ±0.53% ±0.71% ±0.93%
 buffers/buffer-from.js n=2048 len=2048 source='object'                           ***      6.82 %       ±3.30% ±4.44% ±5.88%
 buffers/buffer-from.js n=2048 len=2048 source='string'                                    0.27 %       ±0.66% ±0.88% ±1.16%
 buffers/buffer-from.js n=2048 len=2048 source='string-base64'                            -0.77 %       ±0.99% ±1.33% ±1.77%
 buffers/buffer-from.js n=2048 len=2048 source='string-utf8'                       **      1.08 %       ±0.78% ±1.04% ±1.36%
 buffers/buffer-from.js n=2048 len=2048 source='uint8array'                               -0.29 %       ±0.80% ±1.07% ±1.39%

Be aware that when doing many comparisions the risk of a false-positive
result increases. In this case there are 48 comparisions, you can thus
expect the following amount of false-positive results:
  2.40 false positives, when considering a   5% risk acceptance (*, **, ***),
  0.48 false positives, when considering a   1% risk acceptance (**, ***),
  0.05 false positives, when considering a 0.1% risk acceptance (***)

result of benchmark/stirng_decoder:

                                                                                   confidence improvement accuracy (*)   (**)  (***)
 string_decoder/string-decoder.js n=2500000 chunkLen=64 inLen=4096 encoding='utf8'        ***    -13.45 %       ±1.19% ±1.60% ±2.11%

Be aware that when doing many comparisions the risk of a false-positive
result increases. In this case there are 1 comparisions, you can thus
expect the following amount of false-positive results:
  0.05 false positives, when considering a   5% risk acceptance (*, **, ***),
  0.01 false positives, when considering a   1% risk acceptance (**, ***),
  0.00 false positives, when considering a 0.1% risk acceptance (***)

result of benchmark/url:

                                                                      confidence improvement accuracy (*)   (**)  (***)
 url/whatwg-url-properties.js n=300000 prop='host' input='auth'              ***     -5.72 %       ±1.75% ±2.33% ±3.04%
 url/whatwg-url-properties.js n=300000 prop='host' input='dot'               ***     -7.24 %       ±1.29% ±1.72% ±2.24%
 url/whatwg-url-properties.js n=300000 prop='host' input='file'                       0.63 %       ±1.69% ±2.25% ±2.93%
 url/whatwg-url-properties.js n=300000 prop='host' input='idn'               ***     -6.43 %       ±1.82% ±2.42% ±3.16%
 url/whatwg-url-properties.js n=300000 prop='host' input='javascript'        ***     -6.57 %       ±2.21% ±2.94% ±3.83%
 url/whatwg-url-properties.js n=300000 prop='host' input='long'              ***     -6.05 %       ±1.50% ±2.00% ±2.60%
 url/whatwg-url-properties.js n=300000 prop='host' input='percent'           ***     -6.37 %       ±1.55% ±2.06% ±2.68%
 url/whatwg-url-properties.js n=300000 prop='host' input='short'             ***     -7.85 %       ±1.75% ±2.33% ±3.04%
 url/whatwg-url-properties.js n=300000 prop='host' input='ws'                ***     -5.97 %       ±1.87% ±2.49% ±3.25%

Be aware that when doing many comparisions the risk of a false-positive
result increases. In this case there are 9 comparisions, you can thus
expect the following amount of false-positive results:
  0.45 false positives, when considering a   5% risk acceptance (*, **, ***),
  0.09 false positives, when considering a   1% risk acceptance (**, ***),
  0.01 false positives, when considering a 0.1% risk acceptance (***)

Odd that two of the buffer benchmarks go up by 6 or 7%.

In general the numbers look pretty bad though. A 1-2% drop might have been acceptable but this probably is not.

This is with identical builds except for -fstack-protector-strong?

Yes, I also noticed some test even have performance improve, quite strange.
I think I may need to double check my build.
As -fstack-protector-strong was not supported in clang 3.4.2, so I used -fstack-protector instead.

this and our disabling of v8's untrusted code mitigations make me wonder if we should add like a "more security" option to our configure which enables these things at the cost of performance.

@devsnek

@devsnek Yes, I also think a config option may be another choice for this kind of changes which is more flexible.

@bnoordhuis

While, based on latest findings that the '-fstack-protector-strong' was enabled by default in gcc for ubuntu from 14.10 (please ref here). It would be not good to forcefully use the -fstack-protector in node. I would close this issue.

Thanks for providing comments! @bnoordhuis @devsnek

Navigation Menu

Repository navigation

build: Add Stack-based Buffer Overrun Detection (-fstack-protector) #20928

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

build: Add Stack-based Buffer Overrun Detection (-fstack-protector) #20928

Description

Activity

tingshao commented on Jun 7, 2018

bnoordhuis commented on Jun 7, 2018

tingshao commented on Jun 7, 2018

bnoordhuis commented on Jun 7, 2018

tingshao commented on Jun 8, 2018

devsnek commented on Jun 8, 2018

tingshao commented on Jun 8, 2018

tingshao commented on Jul 2, 2018

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions