buffer: harden validation of buffer allocation size #26162

ZYSzys · 2019-02-17T13:43:13Z

Checklist

make -j4 test (UNIX), or vcbuild test (Windows) passes
tests and/or benchmarks are included
commit message follows commit guidelines

Trott · 2019-02-17T15:00:31Z

In case anyone else wonders (as I did) if using RangeError is appropriate for NaN or if something like TypeError might be more appropriate:

String.fromCodePoint(NaN) returns a RangeError so that would seem to be the established convention. 👍

lib/buffer.js

addaleax · 2019-02-17T16:29:54Z

I’m adding semver-major because this seems like a breaking change to me (slightly odd but consistent behaviour → throwing an exception).

CI: https://ci.nodejs.org/job/node-test-pull-request/20835/

Trott · 2019-02-17T16:59:30Z

I ran the Buffer creation benchmark. TL;DR is they look fine.

                                                                    confidence improvement accuracy (*)   (**)  (***)
 buffers/buffer-creation.js n=1024 len=10 type='buffer()'                           0.10 %       ±2.08% ±2.77% ±3.60%
 buffers/buffer-creation.js n=1024 len=10 type='fast-alloc-fill'                    0.45 %       ±2.30% ±3.06% ±3.99%
 buffers/buffer-creation.js n=1024 len=10 type='fast-alloc'                        -1.07 %       ±2.35% ±3.13% ±4.07%
 buffers/buffer-creation.js n=1024 len=10 type='fast-allocUnsafe'           **     -2.37 %       ±1.63% ±2.17% ±2.82%
 buffers/buffer-creation.js n=1024 len=10 type='slow-allocUnsafe'                  -0.29 %       ±1.96% ±2.60% ±3.39%
 buffers/buffer-creation.js n=1024 len=10 type='slow'                               0.63 %       ±1.88% ±2.51% ±3.26%
 buffers/buffer-creation.js n=1024 len=1024 type='buffer()'                         0.75 %       ±1.42% ±1.90% ±2.47%
 buffers/buffer-creation.js n=1024 len=1024 type='fast-alloc-fill'                  0.35 %       ±1.36% ±1.81% ±2.36%
 buffers/buffer-creation.js n=1024 len=1024 type='fast-alloc'                      -0.29 %       ±1.24% ±1.65% ±2.15%
 buffers/buffer-creation.js n=1024 len=1024 type='fast-allocUnsafe'                -0.13 %       ±2.62% ±3.49% ±4.55%
 buffers/buffer-creation.js n=1024 len=1024 type='slow-allocUnsafe'                -1.13 %       ±1.47% ±1.96% ±2.55%
 buffers/buffer-creation.js n=1024 len=1024 type='slow'                            -0.52 %       ±1.34% ±1.79% ±2.33%
 buffers/buffer-creation.js n=1024 len=2048 type='buffer()'                        -0.03 %       ±0.68% ±0.91% ±1.19%
 buffers/buffer-creation.js n=1024 len=2048 type='fast-alloc-fill'                  0.53 %       ±0.78% ±1.03% ±1.35%
 buffers/buffer-creation.js n=1024 len=2048 type='fast-alloc'                      -0.06 %       ±0.89% ±1.18% ±1.54%
 buffers/buffer-creation.js n=1024 len=2048 type='fast-allocUnsafe'                -1.09 %       ±2.20% ±2.93% ±3.82%
 buffers/buffer-creation.js n=1024 len=2048 type='slow-allocUnsafe'                -1.30 %       ±1.46% ±1.96% ±2.57%
 buffers/buffer-creation.js n=1024 len=2048 type='slow'                            -0.15 %       ±1.68% ±2.24% ±2.91%
 buffers/buffer-creation.js n=1024 len=4096 type='buffer()'                        -0.08 %       ±0.77% ±1.03% ±1.34%
 buffers/buffer-creation.js n=1024 len=4096 type='fast-alloc-fill'                 -0.07 %       ±0.68% ±0.91% ±1.18%
 buffers/buffer-creation.js n=1024 len=4096 type='fast-alloc'                      -0.43 %       ±0.64% ±0.85% ±1.11%
 buffers/buffer-creation.js n=1024 len=4096 type='fast-allocUnsafe'                 0.12 %       ±1.26% ±1.67% ±2.18%
 buffers/buffer-creation.js n=1024 len=4096 type='slow-allocUnsafe'                -1.20 %       ±2.13% ±2.84% ±3.73%
 buffers/buffer-creation.js n=1024 len=4096 type='slow'                            -0.12 %       ±2.14% ±2.85% ±3.71%
 buffers/buffer-creation.js n=1024 len=8192 type='buffer()'                         0.20 %       ±0.61% ±0.82% ±1.07%
 buffers/buffer-creation.js n=1024 len=8192 type='fast-alloc-fill'                  0.11 %       ±0.40% ±0.53% ±0.69%
 buffers/buffer-creation.js n=1024 len=8192 type='fast-alloc'                       0.09 %       ±0.55% ±0.73% ±0.95%
 buffers/buffer-creation.js n=1024 len=8192 type='fast-allocUnsafe'                 0.96 %       ±1.33% ±1.78% ±2.31%
 buffers/buffer-creation.js n=1024 len=8192 type='slow-allocUnsafe'                 1.25 %       ±1.29% ±1.72% ±2.24%
 buffers/buffer-creation.js n=1024 len=8192 type='slow'                             0.30 %       ±1.52% ±2.02% ±2.63%

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

Re-ran the one that came up statistically significant to see if it was a false positive and seems like it was:

                                                                  confidence improvement accuracy (*)   (**)  (***)
 buffers/buffer-creation.js n=1024 len=10 type='fast-allocUnsafe'                -1.80 %       ±3.48% ±4.64% ±6.07%

Be aware that when doing many comparisons the risk of a false-positive
result increases. In this case there are 1 comparisons, 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 (***)

Trott · 2019-02-17T17:22:43Z

And here's the Buffer.concat() benchmark results. TL;DR is they look fine too.

 buffers/buffer-concat.js n=1024 withTotalLength=0 pieceSize=1 pieces=1                   -0.95 %       ±2.14% ±2.86% ±3.74%
 buffers/buffer-concat.js n=1024 withTotalLength=0 pieceSize=1 pieces=16                  -1.15 %       ±1.41% ±1.89% ±2.47%
 buffers/buffer-concat.js n=1024 withTotalLength=0 pieceSize=1 pieces=4                   -0.68 %       ±1.93% ±2.56% ±3.34%
 buffers/buffer-concat.js n=1024 withTotalLength=0 pieceSize=16 pieces=1                   0.80 %       ±2.83% ±3.77% ±4.92%
 buffers/buffer-concat.js n=1024 withTotalLength=0 pieceSize=16 pieces=16                 -0.65 %       ±1.25% ±1.67% ±2.20%
 buffers/buffer-concat.js n=1024 withTotalLength=0 pieceSize=16 pieces=4            *     -1.89 %       ±1.77% ±2.37% ±3.10%
 buffers/buffer-concat.js n=1024 withTotalLength=0 pieceSize=256 pieces=1                  0.44 %       ±2.28% ±3.04% ±3.96%
 buffers/buffer-concat.js n=1024 withTotalLength=0 pieceSize=256 pieces=16                -0.70 %       ±0.81% ±1.09% ±1.42%
 buffers/buffer-concat.js n=1024 withTotalLength=0 pieceSize=256 pieces=4                 -0.57 %       ±1.06% ±1.42% ±1.85%
 buffers/buffer-concat.js n=1024 withTotalLength=1 pieceSize=1 pieces=1                    0.19 %       ±1.91% ±2.55% ±3.33%
 buffers/buffer-concat.js n=1024 withTotalLength=1 pieceSize=1 pieces=16                  -0.83 %       ±1.32% ±1.76% ±2.30%
 buffers/buffer-concat.js n=1024 withTotalLength=1 pieceSize=1 pieces=4                   -1.23 %       ±1.65% ±2.20% ±2.88%
 buffers/buffer-concat.js n=1024 withTotalLength=1 pieceSize=16 pieces=1                   0.34 %       ±3.49% ±4.65% ±6.05%
 buffers/buffer-concat.js n=1024 withTotalLength=1 pieceSize=16 pieces=16                 -0.56 %       ±1.33% ±1.77% ±2.30%
 buffers/buffer-concat.js n=1024 withTotalLength=1 pieceSize=16 pieces=4                   0.30 %       ±1.74% ±2.31% ±3.01%
 buffers/buffer-concat.js n=1024 withTotalLength=1 pieceSize=256 pieces=1                  0.27 %       ±1.52% ±2.04% ±2.67%
 buffers/buffer-concat.js n=1024 withTotalLength=1 pieceSize=256 pieces=16                -0.10 %       ±0.98% ±1.31% ±1.71%
 buffers/buffer-concat.js n=1024 withTotalLength=1 pieceSize=256 pieces=4                 -0.06 %       ±1.06% ±1.42% ±1.87%

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

And a re-run of the likely false-positive:

                                                                        confidence improvement accuracy (*)   (**)  (***)
 buffers/buffer-concat.js n=1024 withTotalLength=0 pieceSize=16 pieces=4                 0.49 %       ±0.96% ±1.28% ±1.67%

Be aware that when doing many comparisons the risk of a false-positive
result increases. In this case there are 1 comparisons, 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 (***)

Trott · 2019-02-17T17:30:18Z

Seems unlikely that this would cause test failures in CITGM, but stranger things have happened. CITGM: https://ci.nodejs.org/view/Node.js-citgm/job/citgm-smoker/1743/

addaleax · 2019-02-17T18:03:19Z

Resume CI: https://ci.nodejs.org/job/node-test-pull-request/20842/ ✅

lib/buffer.js

jorangreef · 2019-02-18T08:08:25Z

If this is going to be semver major, perhaps now might also be a good time to update the docs and change the assertion to assert that size is an integer, not a number. Or otherwise the docs should make clear whether floats are rounded down or what exactly happens when you try Buffer.alloc(1.4999999999999999).

It seems odd that Buffer.alloc() accepts floats for an underlying call that works in terms of integers.

jorangreef · 2019-02-18T08:13:38Z

For example, in the Node console,

> 1.4999999999999999
1.5

and yet

> Buffer.alloc(1.4999999999999999).length
1

and more surprising still

> Buffer.alloc(1.9).length
1

I would have expected Buffer.alloc(1.5) or Buffer.alloc(1.9) to provide enough space for the fractional byte, i.e. Buffer.alloc(1.5) or Buffer.alloc(1.9) should return a buffer with enough space for 2 bytes, to be safe, rather than risk overflow.

addaleax · 2019-02-18T08:21:26Z

CI: https://ci.nodejs.org/job/node-test-pull-request/20864/

ZYSzys · 2019-02-18T11:55:24Z

Re-ran buffer creation benchmark, it almost looks fine.


                                                                    confidence improvement accuracy (*)    (**)   (***)
 buffers/buffer-creation.js n=1024 len=10 type='buffer()'                           1.34 %       ±5.24%  ±6.97%  ±9.09%
 buffers/buffer-creation.js n=1024 len=10 type='fast-alloc-fill'                    4.11 %       ±7.17%  ±9.54% ±12.43%
 buffers/buffer-creation.js n=1024 len=10 type='fast-alloc'                         1.65 %       ±5.17%  ±6.88%  ±8.95%
 buffers/buffer-creation.js n=1024 len=10 type='fast-allocUnsafe'                   1.53 %      ±13.85% ±18.43% ±24.00%
 buffers/buffer-creation.js n=1024 len=10 type='slow-allocUnsafe'                   0.87 %       ±6.03%  ±8.03% ±10.45%
 buffers/buffer-creation.js n=1024 len=10 type='slow'                              -3.52 %       ±5.59%  ±7.44%  ±9.68%
 buffers/buffer-creation.js n=1024 len=1024 type='buffer()'                  *     -4.91 %       ±4.68%  ±6.25%  ±8.17%
 buffers/buffer-creation.js n=1024 len=1024 type='fast-alloc-fill'                  1.20 %       ±7.32%  ±9.75% ±12.69%
 buffers/buffer-creation.js n=1024 len=1024 type='fast-alloc'                       0.08 %       ±6.59%  ±8.77% ±11.42%
 buffers/buffer-creation.js n=1024 len=1024 type='fast-allocUnsafe'                 4.81 %      ±11.97% ±15.94% ±20.76%
 buffers/buffer-creation.js n=1024 len=1024 type='slow-allocUnsafe'                -1.92 %       ±7.28%  ±9.68% ±12.61%
 buffers/buffer-creation.js n=1024 len=1024 type='slow'                            -3.22 %       ±6.79%  ±9.05% ±11.80%
 buffers/buffer-creation.js n=1024 len=2048 type='buffer()'                         0.34 %       ±6.36%  ±8.47% ±11.03%
 buffers/buffer-creation.js n=1024 len=2048 type='fast-alloc-fill'                 -0.15 %       ±8.58% ±11.42% ±14.87%
 buffers/buffer-creation.js n=1024 len=2048 type='fast-alloc'                      -0.03 %       ±5.38%  ±7.17%  ±9.33%
 buffers/buffer-creation.js n=1024 len=2048 type='fast-allocUnsafe'                 2.38 %       ±9.37% ±12.47% ±16.25%
 buffers/buffer-creation.js n=1024 len=2048 type='slow-allocUnsafe'                -0.36 %       ±7.32%  ±9.74% ±12.67%
 buffers/buffer-creation.js n=1024 len=2048 type='slow'                             1.19 %       ±4.78%  ±6.37%  ±8.29%
 buffers/buffer-creation.js n=1024 len=4096 type='buffer()'                         0.20 %       ±6.43%  ±8.55% ±11.14%
 buffers/buffer-creation.js n=1024 len=4096 type='fast-alloc-fill'                  0.15 %       ±6.74%  ±8.96% ±11.67%
 buffers/buffer-creation.js n=1024 len=4096 type='fast-alloc'                       1.04 %       ±5.79%  ±7.71% ±10.06%
 buffers/buffer-creation.js n=1024 len=4096 type='fast-allocUnsafe'                 1.68 %       ±9.00% ±11.97% ±15.58%
 buffers/buffer-creation.js n=1024 len=4096 type='slow-allocUnsafe'                 1.29 %       ±6.87%  ±9.14% ±11.89%
 buffers/buffer-creation.js n=1024 len=4096 type='slow'                             3.23 %       ±7.16%  ±9.53% ±12.41%
 buffers/buffer-creation.js n=1024 len=8192 type='buffer()'                        -1.04 %       ±7.51% ±10.00% ±13.01%
 buffers/buffer-creation.js n=1024 len=8192 type='fast-alloc-fill'                 -0.58 %       ±7.50%  ±9.98% ±12.99%
 buffers/buffer-creation.js n=1024 len=8192 type='fast-alloc'                       2.00 %       ±7.45%  ±9.91% ±12.91%
 buffers/buffer-creation.js n=1024 len=8192 type='fast-allocUnsafe'                 2.14 %       ±9.38% ±12.49% ±16.25%
 buffers/buffer-creation.js n=1024 len=8192 type='slow-allocUnsafe'                 0.26 %       ±6.79%  ±9.04% ±11.77%
 buffers/buffer-creation.js n=1024 len=8192 type='slow'                             0.19 %       ±7.36%  ±9.80% ±12.75%
Be aware that when doing many comparisons the risk of a false-positive

result increases. In this case there are 30 comparisons, you can thus

expect the following amount of false-positive results:

1.50 false positives, when considering a   5% risk acceptance (*, **, *),

0.30 false positives, when considering a   1% risk acceptance (, ),

0.03 false positives, when considering a 0.1% risk acceptance ()

addaleax · 2019-02-21T21:11:14Z

Landed in 6fb7baf

This makes using `NaN` as the buffer size throw an error. Fixes: #26151 PR-URL: #26162 Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Сковорода Никита Андреевич <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]>

Notable changes: * assert: * improve performance to instantiate errors (Ruben Bridgewater) [#26738](#26738) * validate required arguments (Ruben Bridgewater) [#26641](#26641) * adjust loose assertions (Ruben Bridgewater) [#25008](#25008) * async_hooks: * remove deprecated emitBefore and emitAfter (Matteo Collina) [#26530](#26530) * remove promise object from resource (Andreas Madsen) [#23443](#23443) * bootstrap * make Buffer and process non-enumerable (Ruben Bridgewater) [#24874](#24874) * buffer: * use stricter range checks (Ruben Bridgewater) [#27045](#27045) * harden SlowBuffer creation (ZYSzys) [#26272](#26272) * harden validation of buffer allocation size (ZYSzys) [#26162](#26162) * do proper error propagation in addon methods (Anna Henningsen) [#23939](#23939) * child_process: * change the defaults maxBuffer size (kohta ito) [#27179](#27179) * harden fork arguments validation (ZYSzys) [#27039](#27039) * use non-infinite maxBuffer defaults (kohta ito) [#23027](#23027) * console: * don't use ANSI escape codes when TERM=dumb (Vladislav Kaminsky) [#26261](#26261) * crypto: * remove legacy native handles (Tobias Nießen) [#27011](#27011) * decode missing passphrase errors (Tobias Nießen) [#25208](#25208) * move DEP0113 to End-of-Life (Tobias Nießen) [#26249](#26249) * remove deprecated crypto.\_toBuf (Tobias Nießen) [#25338](#25338) * set `DEFAULT\_ENCODING` property to non-enumerable (Antoine du Hamel) [#23222](#23222) * deps: * silence irrelevant V8 warning (Michaël Zasso) [#26685](#26685) * update postmortem metadata generation script (cjihrig) [#26685](#26685) * V8: un-cherry-pick bd019bd (Refael Ackermann) [#26685](#26685) * V8: cherry-pick 6 commits (Michaël Zasso) [#26685](#26685) * V8: cherry-pick d82c9af (Anna Henningsen) [#26685](#26685) * V8: cherry-pick e5f01ba (Anna Henningsen) [#26685](#26685) * V8: cherry-pick d5f08e4 (Anna Henningsen) [#26685](#26685) * V8: cherry-pick 6b09d21 (Anna Henningsen) [#26685](#26685) * V8: cherry-pick f0bb5d2 (Anna Henningsen) [#26685](#26685) * V8: cherry-pick 5b0510d (Anna Henningsen) [#26685](#26685) * V8: cherry-pick 91f0cd0 (Anna Henningsen) [#26685](#26685) * V8: cherry-pick 392316d (Anna Henningsen) [#26685](#26685) * V8: cherry-pick 2f79d68 (Anna Henningsen) [#26685](#26685) * sync V8 gypfiles with 7.4 (Ujjwal Sharma) [#26685](#26685) * update V8 to 7.4.288.13 (Ujjwal Sharma) [#26685](#26685) * bump minimum icu version to 63 (Ujjwal Sharma) [#25852](#25852) * silence irrelevant V8 warnings (Michaël Zasso) [#25852](#25852) * V8: cherry-pick 7803fa6 (Jon Kunkee) [#25852](#25852) * V8: cherry-pick 58cefed (Jon Kunkee) [#25852](#25852) * V8: cherry-pick d3308d0 (Michaël Zasso) [#25852](#25852) * V8: cherry-pick 74571c8 (Michaël Zasso) [#25852](#25852) * cherry-pick fc0ddf5 from upstream V8 (Anna Henningsen) [#25852](#25852) * sync V8 gypfiles with 7.3 (Ujjwal Sharma) [#25852](#25852) * sync V8 gypfiles with 7.2 (Michaël Zasso) [#25852](#25852) * update V8 to 7.3.492.25 (Michaël Zasso) [#25852](#25852) * add s390 asm rules for OpenSSL-1.1.1 (Shigeki Ohtsu) [#19794](#19794) * sync V8 gypfiles with 7.1 (Refael Ackermann) [#23423](#23423) * update V8 to 7.1.302.28 (Michaël Zasso) [#23423](#23423) * doc: * update behaviour of fs.writeFile (Sakthipriyan Vairamani (thefourtheye)) [#25080](#25080) * add internal functionality details of util.inherits (Ruben Bridgewater) [#24755](#24755) * errors: * update error name (Ruben Bridgewater) [#26738](#26738) * fs: * use proper .destroy() implementation for SyncWriteStream (Matteo Collina) [#26690](#26690) * improve mode validation (Ruben Bridgewater) [#26575](#26575) * harden validation of start option in createWriteStream (ZYSzys) [#25579](#25579) * make writeFile consistent with readFile wrt fd (Sakthipriyan Vairamani (thefourtheye)) [#23709](#23709) * http: * validate timeout in ClientRequest() (cjihrig) [#26214](#26214) * return HTTP 431 on HPE\_HEADER\_OVERFLOW error (Albert Still) [#25605](#25605) * switch default parser to llhttp (Anna Henningsen) [#24870](#24870) * change DEP0066 to a runtime deprecation (Morgan Roderick) [#24167](#24167) * else case is not reachable (szabolcsit) [#24176](#24176) * lib: * move DEP0021 to end of life (cjihrig) [#27127](#27127) * remove Atomics.wake (Gus Caplan) [#27033](#27033) * validate Error.captureStackTrace() calls (Ruben Bridgewater) [#26738](#26738) * refactor Error.captureStackTrace() usage (Ruben Bridgewater) [#26738](#26738) * move DTRACE\_\* probes out of global scope (James M Snell) [#26541](#26541) * deprecate \_stream\_wrap (Sam Roberts) [#26245] (#26245) * don't use `util.inspect()` internals (Ruben Bridgewater) [#24971](#24971) * improve error message for MODULE\_NOT\_FOUND (Ali Ijaz Sheikh) [#25690](#25690) * requireStack property for MODULE\_NOT\_FOUND (Ali Ijaz Sheikh) [#25690](#25690) * move DEP0029 to end of life (cjihrig) [#25377](#25377) * move DEP0028 to end of life (cjihrig) [#25377](#25377) * move DEP0027 to end of life (cjihrig) [#25377](#25377) * move DEP0026 to end of life (cjihrig) [#25377](#25377) * move DEP0023 to end of life (cjihrig) [#25280](#25280) * move DEP0006 to end of life (cjihrig) [#25279](#25279) * remove unintended access to deps/ (Anna Henningsen) [#25138](#25138) * move DEP0120 to end of life (cjihrig) [#24862](#24862) * use ES6 class inheritance style (Ruben Bridgewater) [#24755](#24755) * remove `inherits()` usage (Ruben Bridgewater) [#24755](#24755) * module: * remove dead code (Ruben Bridgewater) [#26983](#26983) * mark DEP0019 as End-of-Life (Ruben Bridgewater) [#26973](#26973) * throw an error for invalid package.json main entries (Ruben Bridgewater) [#26823](#26823) * don't search in require.resolve.paths (cjihrig) [#23683](#23683) * n-api: * remove code from error name (Ruben Bridgewater) [#26738](#26738) * net: * do not manipulate potential user code (Ruben Bridgewater) [#26751](#26751) * emit "write after end" errors in the next tick (Ouyang Yadong) [#24457](#24457) * deprecate \_setSimultaneousAccepts() undocumented function (James M Snell) [#23760](#23760) * net,http2: * merge setTimeout code (ZYSzys) [#25084](#25084) * os: * implement os.type() using uv\_os\_uname() (cjihrig) [#25659](#25659) * process: * global.process, global.Buffer getters (Guy Bedford) [#26882](#26882) * move DEP0062 (node --debug) to end-of-life (Joyee Cheung) [#25828](#25828) * exit on --debug and --debug-brk after option parsing (Joyee Cheung) [#25828](#25828) * improve `--redirect-warnings` handling (Ruben Bridgewater) [#24965](#24965) * readline: * support TERM=dumb (Vladislav Kaminsky) [#26261](#26261) * repl: * add welcome message (gengjiawen) [#25947](#25947) * fix terminal default setting (Ruben Bridgewater) [#26518](#26518) * check colors with .getColorDepth() (Vladislav Kaminsky) [#26261](#26261) * deprecate REPLServer.rli (Ruben Bridgewater) [#26260](#26260) * src: * remove unused INT\_MAX constant (Sam Roberts) [#27078](#27078) * update NODE\_MODULE\_VERSION to 72 (Ujjwal Sharma) [#26685](#26685) * remove `AddPromiseHook()` (Anna Henningsen) [#26574](#26574) * update NODE\_MODULE\_VERSION to 71 (Michaël Zasso) [#25852](#25852) * clean up MultiIsolatePlatform interface (Anna Henningsen) [#26384](#26384) * properly configure default heap limits (Ali Ijaz Sheikh) [#25576](#25576) * remove icuDataDir from node config (GauthamBanasandra) [#24780](#24780) * explicitly allow JS in ReadHostObject (Yang Guo) [#23423](#23423) * update postmortem constant (cjihrig) [#23423](#23423) * update NODE\_MODULE\_VERSION to 68 (Michaël Zasso) [#23423](#23423) * tls: * support TLSv1.3 (Sam Roberts) [#26209](#26209) * return correct version from getCipher() (Sam Roberts) [#26625](#26625) * check arg types of renegotiate() (Sam Roberts) [#25876](#25876) * add code for ERR\_TLS\_INVALID\_PROTOCOL\_METHOD (Sam Roberts) [#24729](#24729) * emit a warning when servername is an IP address (Rodger Combs) [#23329](#23329) * disable TLS v1.0 and v1.1 by default (Ben Noordhuis) [#23814](#23814) * remove unused arg to createSecureContext() (Sam Roberts) [#24241](#24241) * deprecate Server.prototype.setOptions() (cjihrig)[ #23820](#23820) * load NODE\_EXTRA\_CA\_CERTS at startup (Ouyang Yadong) [#23354](#23354) * util: * change inspect compact and breakLength default (Ruben Bridgewater) [#27109](#27109) * improve inspect edge cases (Ruben Bridgewater) [#27109](#27109) * only the first line of the error message (Simon Zünd) [#26685](#26685) * don't set the prototype of callbackified functions (Ruben Bridgewater) [#26893](#26893) * rename callbackified function (Ruben Bridgewater) [#26893](#26893) * increase function length when using `callbackify()` (Ruben Bridgewater) [#26893](#26893) * prevent tampering with internals in `inspect()` (Ruben Bridgewater) [#26577](#26577) * fix proxy inspection (Ruben Bridgewater) [#26241](#26241) * prevent leaking internal properties (Ruben Bridgewater) [#24971](#24971) * protect against monkeypatched Object prototype for inspect() (Rich Trott) [#25953](#25953) * treat format arguments equally (Roman Reiss) [#23162](#23162) * win, fs: * detect if symlink target is a directory (Bartosz Sosnowski) [#23724](#23724) * zlib: * throw TypeError if callback is missing (Anna Henningsen)[ #24929](#24929) * make “bare” constants un-enumerable (Anna Henningsen) [#24824](#24824) PR-URL: #26930

Notable changes: * assert: * validate required arguments (Ruben Bridgewater) [#26641](#26641) * adjust loose assertions (Ruben Bridgewater) [#25008](#25008) * async_hooks: * remove deprecated `emitBefore` and `emitAfter` (Matteo Collina) [#26530](#26530) * remove promise object from resource (Andreas Madsen) [#23443](#23443) * bootstrap: make Buffer and process non-enumerable (Ruben Bridgewater) [#24874](#24874) * buffer: * use stricter range checks (Ruben Bridgewater) [#27045](#27045) * harden `SlowBuffer` creation (ZYSzys) [#26272](#26272) * harden validation of buffer allocation size (ZYSzys) [#26162](#26162) * do proper error propagation in addon methods (Anna Henningsen) [#23939](#23939) * child_process: * remove `options.customFds` (cjihrig) [#25279](#25279) * harden fork arguments validation (ZYSzys) [#27039](#27039) * use non-infinite `maxBuffer` defaults (kohta ito) [#23027](#23027) * console: * don't use ANSI escape codes when `TERM=dumb` (Vladislav Kaminsky) [#26261](#26261) * crypto: * remove legacy native handles (Tobias Nießen) [#27011](#27011) * decode missing passphrase errors (Tobias Nießen) [#25208](#25208) * remove `Cipher.setAuthTag()` and `Decipher.getAuthTag()` (Tobias Nießen) [#26249](#26249) * remove deprecated `crypto._toBuf()` (Tobias Nießen) [#25338](#25338) * set `DEFAULT\_ENCODING` property to non-enumerable (Antoine du Hamel) [#23222](#23222) * deps: * update V8 to 7.4.288.13 (Michaël Zasso, cjihrig, Refael Ackermann) (Anna Henningsen, Ujjwal Sharma) [#26685](#26685) * bump minimum icu version to 63 (Ujjwal Sharma) [#25852](#25852) * update OpenSSL to 1.1.1b (Sam Roberts, Shigeki Ohtsu) [#26327](#26327) * errors: * update error name (Ruben Bridgewater) [#26738](#26738) * fs: * use proper .destroy() implementation for SyncWriteStream (Matteo Collina) [#26690](#26690) * improve mode validation (Ruben Bridgewater) [#26575](#26575) * harden validation of start option in `createWriteStream()` (ZYSzys) [#25579](#25579) * make writeFile consistent with readFile wrt fd (Sakthipriyan Vairamani (thefourtheye)) [#23709](#23709) * http: * validate timeout in `ClientRequest()` (cjihrig) [#26214](#26214) * return HTTP 431 on `HPE_HEADER_OVERFLOW` error (Albert Still) [#25605](#25605) * switch default parser to llhttp (Anna Henningsen) [#24870](#24870) * Runtime-deprecate `outgoingMessage._headers` and `outgoingMessage._headerNames` (Morgan Roderick) [#24167](#24167) * lib: * remove `Atomics.wake()` (Gus Caplan) [#27033](#27033) * move DTRACE\_\* probes out of global scope (James M Snell) [#26541](#26541) * deprecate `_stream_wrap` (Sam Roberts) [#26245](#26245) * use ES6 class inheritance style (Ruben Bridgewater) [#24755](#24755) * module: * remove unintended access to deps/ (Anna Henningsen) [#25138](#25138) * improve error message for MODULE\_NOT\_FOUND (Ali Ijaz Sheikh) [#25690](#25690) * requireStack property for MODULE\_NOT\_FOUND (Ali Ijaz Sheikh) [#25690](#25690) * remove dead code (Ruben Bridgewater) [#26983](#26983) * make `require('.')` never resolve outside the current directory (Ruben Bridgewater) [#26973](#26973) * throw an error for invalid package.json main entries (Ruben Bridgewater) [#26823](#26823) * don't search in `require.resolve.paths` (cjihrig) [#23683](#23683) * net: * remove `Server.listenFD()` (cjihrig) [#27127](#27127) * do not add `.host` and `.port` properties to DNS error (Ruben Bridgewater) [#26751](#26751) * emit "write after end" errors in the next tick (Ouyang Yadong) [#24457](#24457) * deprecate `_setSimultaneousAccepts()` undocumented function (James M Snell) [#23760](#23760) * os: * implement `os.type()` using `uv_os_uname()` (cjihrig) [#25659](#25659) * remove `os.getNetworkInterfaces()` (cjihrig) [#25280](#25280) * process: * make global.process, global.Buffer getters (Guy Bedford) [#26882](#26882) * move DEP0062 (node --debug) to end-of-life (Joyee Cheung) [#25828](#25828) * exit on --debug and --debug-brk after option parsing (Joyee Cheung) [#25828](#25828) * improve `--redirect-warnings` handling (Ruben Bridgewater) [#24965](#24965) * readline: * support TERM=dumb (Vladislav Kaminsky) [#26261](#26261) * repl: * add welcome message (gengjiawen) [#25947](#25947) * fix terminal default setting (Ruben Bridgewater) [#26518](#26518) * check colors with `.getColorDepth()` (Vladislav Kaminsky) [#26261](#26261) * deprecate REPLServer.rli (Ruben Bridgewater) [#26260](#26260) * src: * remove unused `INT_MAX` constant (Sam Roberts) [#27078](#27078) * update `NODE_MODULE_VERSION` to 72 (Ujjwal Sharma) [#26685](#26685) * remove `AddPromiseHook()` (Anna Henningsen) [#26574](#26574) * clean up `MultiIsolatePlatform` interface (Anna Henningsen) [#26384](#26384) * properly configure default heap limits (Ali Ijaz Sheikh) [#25576](#25576) * remove `icuDataDir` from node config (GauthamBanasandra) [#24780](#24780) * tls: * support TLSv1.3 (Sam Roberts) [#26209](#26209) * return correct version from `getCipher()` (Sam Roberts) [#26625](#26625) * check arg types of renegotiate() (Sam Roberts) [#25876](#25876) * add code for `ERR_TLS_INVALID_PROTOCOL_METHOD` (Sam Roberts) [#24729](#24729) * emit a warning when servername is an IP address (Rodger Combs) [#23329](#23329) * disable TLS v1.0 and v1.1 by default (Ben Noordhuis) [#23814](#23814) * remove unused arg to createSecureContext() (Sam Roberts) [#24241](#24241) * deprecate `Server.prototype.setOptions()` (cjihrig) [#23820](#23820) * load `NODE_EXTRA_CA_CERTS` at startup (Ouyang Yadong) [#23354](#23354) * util: * remove `util.print()`, `util.puts()`, `util.debug()` and `util.error()` (cjihrig) [#25377](#25377) * change inspect compact and breakLength default (Ruben Bridgewater) [#27109](#27109) * improve inspect edge cases (Ruben Bridgewater) [#27109](#27109) * only the first line of the error message (Simon Zünd) [#26685](#26685) * don't set the prototype of callbackified functions (Ruben Bridgewater) [#26893](#26893) * rename callbackified function (Ruben Bridgewater) [#26893](#26893) * increase function length when using `callbackify()` (Ruben Bridgewater) [#26893](#26893) * prevent tampering with internals in `inspect()` (Ruben Bridgewater) [#26577](#26577) * prevent Proxy traps being triggered by `.inspect()` (Ruben Bridgewater) [#26241](#26241) * prevent leaking internal properties (Ruben Bridgewater) [#24971](#24971) * protect against monkeypatched Object prototype for inspect() (Rich Trott) [#25953](#25953) * treat format arguments equally (Roman Reiss) [#23162](#23162) * win, fs: * detect if symlink target is a directory (Bartosz Sosnowski) [#23724](#23724) * zlib: * throw TypeError if callback is missing (Anna Henningsen) [#24929](#24929) * make “bare” constants un-enumerable (Anna Henningsen) [#24824](#24824) PR-URL: #26930

nodejs-github-bot added the buffer Issues and PRs related to the buffer subsystem. label Feb 17, 2019

buffer: harden validation of buffer allocation size

a0628de

Fixes: nodejs#26151

ZYSzys force-pushed the buffer-validate-size branch from 8ee20e2 to a0628de Compare February 17, 2019 14:00

Trott reviewed Feb 17, 2019

View reviewed changes

lib/buffer.js Show resolved Hide resolved

Trott approved these changes Feb 17, 2019

View reviewed changes

addaleax approved these changes Feb 17, 2019

View reviewed changes

addaleax added the semver-major PRs that contain breaking changes and should be released in the next major version. label Feb 17, 2019

lpinca approved these changes Feb 17, 2019

View reviewed changes

jorangreef reviewed Feb 18, 2019

View reviewed changes

lib/buffer.js Outdated Show resolved Hide resolved

buffer: use within range check for buffer alloc size

58b832f

ChALkeR approved these changes Feb 18, 2019

View reviewed changes

jasnell approved these changes Feb 18, 2019

View reviewed changes

BridgeAR approved these changes Feb 20, 2019

View reviewed changes

BridgeAR added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Feb 20, 2019

addaleax mentioned this pull request Feb 21, 2019

Buffer.alloc(): A TypeError will be thrown if size is not a number. #26151

Closed

addaleax closed this Feb 21, 2019

ZYSzys deleted the buffer-validate-size branch February 22, 2019 05:52

ZYSzys mentioned this pull request Feb 23, 2019

buffer: harden SlowBuffer creation #26272

Closed

3 tasks

BethGriggs mentioned this pull request Mar 26, 2019

v12.0.0 proposal #26930

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

buffer: harden validation of buffer allocation size #26162

buffer: harden validation of buffer allocation size #26162

ZYSzys commented Feb 17, 2019

Trott commented Feb 17, 2019

addaleax commented Feb 17, 2019

Trott commented Feb 17, 2019 •

edited

Loading

Trott commented Feb 17, 2019 •

edited

Loading

Trott commented Feb 17, 2019

addaleax commented Feb 17, 2019 •

edited by Trott

Loading

jorangreef commented Feb 18, 2019

jorangreef commented Feb 18, 2019

addaleax commented Feb 18, 2019

ZYSzys commented Feb 18, 2019

addaleax commented Feb 21, 2019

buffer: harden validation of buffer allocation size #26162

buffer: harden validation of buffer allocation size #26162

Conversation

ZYSzys commented Feb 17, 2019

Checklist

Trott commented Feb 17, 2019

addaleax commented Feb 17, 2019

Trott commented Feb 17, 2019 • edited Loading

Trott commented Feb 17, 2019 • edited Loading

Trott commented Feb 17, 2019

addaleax commented Feb 17, 2019 • edited by Trott Loading

jorangreef commented Feb 18, 2019

jorangreef commented Feb 18, 2019

addaleax commented Feb 18, 2019

ZYSzys commented Feb 18, 2019

addaleax commented Feb 21, 2019

Trott commented Feb 17, 2019 •

edited

Loading

Trott commented Feb 17, 2019 •

edited

Loading

addaleax commented Feb 17, 2019 •

edited by Trott

Loading