Skip to content

buffer: add Buffer.harden() method

Rodrigo Muino Tomonari requested to merge github/fork/ChALkeR/harden-buffer into master

This is a strawperson currently, targeted at resolving Buffer controversies and providing an alternate safer Buffer API that could be opted in from the top level app code. This should both resolve potential concerns about Buffer unsafety (that are otherwise non-resolvable in general for performance reasons) and give ecosystem another nudge to migrate off unsafe deprecated Buffer(arg) API.

This adds Buffer.harden() method that:

  1. Ensures all new buffers are zero-filled, even from Buffer.allocUnsafe() (similar to --zero-fill-buffers CLI flag).
  2. Ensures that pooling is permanently disabled.
  3. Enables runtime deprecation for deprecated Buffer(arg)
  4. Freezes Buffer object.

That method is callable only from the top-level app, it will throw when called from dependencies (reuses already existing isInsideNodeModules() check for that).

Example:

> Buffer.allocUnsafe(10)
<Buffer 80 26 00 03 01 00 00 00 00 00>
> Buffer.from('sdfds').buffer
ArrayBuffer {
  [Uint8Contents]: <80 26 00 03 01 00 00 00 00 00 00 00 00 00 00 00 73 64 66 64 73 00 00 00 80 00 10 03 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ff ff ff ff 0a 00 00 00 ff ff ff ff 32 00 00 00 ff ff ff ff 38 00 00 00 80 26 00 03 01 00 00 00 00 00 00 00 00 00 00 00 38 24 81 03 01 00 00 00 00 00 00 00 ... 8092 more bytes>,
  byteLength: 8192
}
> Buffer.harden()
undefined
> Buffer.allocUnsafe(10)
<Buffer 00 00 00 00 00 00 00 00 00 00>
> Buffer.from('sdfds').buffer
ArrayBuffer { [Uint8Contents]: <73 64 66 64 73>, byteLength: 5 }
>

Unresolved:

  1. Re-evaluating Buffer module could create non-hardened Buffers?
  2. Child process Buffers are non hardened -- I thought about going into those by an env var, but let's keep that out of scope

/cc @nodejs/security-wg @nodejs/security @nodejs/buffer @mafintosh -- comments?

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines

Merge request reports