[6a3a178] | 1 | # Porting to the Buffer.from/Buffer.alloc API
|
---|
| 2 |
|
---|
| 3 | <a id="overview"></a>
|
---|
| 4 | ## Overview
|
---|
| 5 |
|
---|
| 6 | - [Variant 1: Drop support for Node.js ≤ 4.4.x and 5.0.0 — 5.9.x.](#variant-1) (*recommended*)
|
---|
| 7 | - [Variant 2: Use a polyfill](#variant-2)
|
---|
| 8 | - [Variant 3: manual detection, with safeguards](#variant-3)
|
---|
| 9 |
|
---|
| 10 | ### Finding problematic bits of code using grep
|
---|
| 11 |
|
---|
| 12 | Just run `grep -nrE '[^a-zA-Z](Slow)?Buffer\s*\(' --exclude-dir node_modules`.
|
---|
| 13 |
|
---|
| 14 | It will find all the potentially unsafe places in your own code (with some considerably unlikely
|
---|
| 15 | exceptions).
|
---|
| 16 |
|
---|
| 17 | ### Finding problematic bits of code using Node.js 8
|
---|
| 18 |
|
---|
| 19 | If you’re using Node.js ≥ 8.0.0 (which is recommended), Node.js exposes multiple options that help with finding the relevant pieces of code:
|
---|
| 20 |
|
---|
| 21 | - `--trace-warnings` will make Node.js show a stack trace for this warning and other warnings that are printed by Node.js.
|
---|
| 22 | - `--trace-deprecation` does the same thing, but only for deprecation warnings.
|
---|
| 23 | - `--pending-deprecation` will show more types of deprecation warnings. In particular, it will show the `Buffer()` deprecation warning, even on Node.js 8.
|
---|
| 24 |
|
---|
| 25 | You can set these flags using an environment variable:
|
---|
| 26 |
|
---|
| 27 | ```console
|
---|
| 28 | $ export NODE_OPTIONS='--trace-warnings --pending-deprecation'
|
---|
| 29 | $ cat example.js
|
---|
| 30 | 'use strict';
|
---|
| 31 | const foo = new Buffer('foo');
|
---|
| 32 | $ node example.js
|
---|
| 33 | (node:7147) [DEP0005] DeprecationWarning: The Buffer() and new Buffer() constructors are not recommended for use due to security and usability concerns. Please use the new Buffer.alloc(), Buffer.allocUnsafe(), or Buffer.from() construction methods instead.
|
---|
| 34 | at showFlaggedDeprecation (buffer.js:127:13)
|
---|
| 35 | at new Buffer (buffer.js:148:3)
|
---|
| 36 | at Object.<anonymous> (/path/to/example.js:2:13)
|
---|
| 37 | [... more stack trace lines ...]
|
---|
| 38 | ```
|
---|
| 39 |
|
---|
| 40 | ### Finding problematic bits of code using linters
|
---|
| 41 |
|
---|
| 42 | Eslint rules [no-buffer-constructor](https://eslint.org/docs/rules/no-buffer-constructor)
|
---|
| 43 | or
|
---|
| 44 | [node/no-deprecated-api](https://github.com/mysticatea/eslint-plugin-node/blob/master/docs/rules/no-deprecated-api.md)
|
---|
| 45 | also find calls to deprecated `Buffer()` API. Those rules are included in some pre-sets.
|
---|
| 46 |
|
---|
| 47 | There is a drawback, though, that it doesn't always
|
---|
| 48 | [work correctly](https://github.com/chalker/safer-buffer#why-not-safe-buffer) when `Buffer` is
|
---|
| 49 | overriden e.g. with a polyfill, so recommended is a combination of this and some other method
|
---|
| 50 | described above.
|
---|
| 51 |
|
---|
| 52 | <a id="variant-1"></a>
|
---|
| 53 | ## Variant 1: Drop support for Node.js ≤ 4.4.x and 5.0.0 — 5.9.x.
|
---|
| 54 |
|
---|
| 55 | This is the recommended solution nowadays that would imply only minimal overhead.
|
---|
| 56 |
|
---|
| 57 | The Node.js 5.x release line has been unsupported since July 2016, and the Node.js 4.x release line reaches its End of Life in April 2018 (→ [Schedule](https://github.com/nodejs/Release#release-schedule)). This means that these versions of Node.js will *not* receive any updates, even in case of security issues, so using these release lines should be avoided, if at all possible.
|
---|
| 58 |
|
---|
| 59 | What you would do in this case is to convert all `new Buffer()` or `Buffer()` calls to use `Buffer.alloc()` or `Buffer.from()`, in the following way:
|
---|
| 60 |
|
---|
| 61 | - For `new Buffer(number)`, replace it with `Buffer.alloc(number)`.
|
---|
| 62 | - For `new Buffer(string)` (or `new Buffer(string, encoding)`), replace it with `Buffer.from(string)` (or `Buffer.from(string, encoding)`).
|
---|
| 63 | - For all other combinations of arguments (these are much rarer), also replace `new Buffer(...arguments)` with `Buffer.from(...arguments)`.
|
---|
| 64 |
|
---|
| 65 | Note that `Buffer.alloc()` is also _faster_ on the current Node.js versions than
|
---|
| 66 | `new Buffer(size).fill(0)`, which is what you would otherwise need to ensure zero-filling.
|
---|
| 67 |
|
---|
| 68 | Enabling eslint rule [no-buffer-constructor](https://eslint.org/docs/rules/no-buffer-constructor)
|
---|
| 69 | or
|
---|
| 70 | [node/no-deprecated-api](https://github.com/mysticatea/eslint-plugin-node/blob/master/docs/rules/no-deprecated-api.md)
|
---|
| 71 | is recommended to avoid accidential unsafe Buffer API usage.
|
---|
| 72 |
|
---|
| 73 | There is also a [JSCodeshift codemod](https://github.com/joyeecheung/node-dep-codemod#dep005)
|
---|
| 74 | for automatically migrating Buffer constructors to `Buffer.alloc()` or `Buffer.from()`.
|
---|
| 75 | Note that it currently only works with cases where the arguments are literals or where the
|
---|
| 76 | constructor is invoked with two arguments.
|
---|
| 77 |
|
---|
| 78 | _If you currently support those older Node.js versions and dropping them would be a semver-major change
|
---|
| 79 | for you, or if you support older branches of your packages, consider using [Variant 2](#variant-2)
|
---|
| 80 | or [Variant 3](#variant-3) on older branches, so people using those older branches will also receive
|
---|
| 81 | the fix. That way, you will eradicate potential issues caused by unguarded Buffer API usage and
|
---|
| 82 | your users will not observe a runtime deprecation warning when running your code on Node.js 10._
|
---|
| 83 |
|
---|
| 84 | <a id="variant-2"></a>
|
---|
| 85 | ## Variant 2: Use a polyfill
|
---|
| 86 |
|
---|
| 87 | Utilize [safer-buffer](https://www.npmjs.com/package/safer-buffer) as a polyfill to support older
|
---|
| 88 | Node.js versions.
|
---|
| 89 |
|
---|
| 90 | You would take exacly the same steps as in [Variant 1](#variant-1), but with a polyfill
|
---|
| 91 | `const Buffer = require('safer-buffer').Buffer` in all files where you use the new `Buffer` api.
|
---|
| 92 |
|
---|
| 93 | Make sure that you do not use old `new Buffer` API — in any files where the line above is added,
|
---|
| 94 | using old `new Buffer()` API will _throw_. It will be easy to notice that in CI, though.
|
---|
| 95 |
|
---|
| 96 | Alternatively, you could use [buffer-from](https://www.npmjs.com/package/buffer-from) and/or
|
---|
| 97 | [buffer-alloc](https://www.npmjs.com/package/buffer-alloc) [ponyfills](https://ponyfill.com/) —
|
---|
| 98 | those are great, the only downsides being 4 deps in the tree and slightly more code changes to
|
---|
| 99 | migrate off them (as you would be using e.g. `Buffer.from` under a different name). If you need only
|
---|
| 100 | `Buffer.from` polyfilled — `buffer-from` alone which comes with no extra dependencies.
|
---|
| 101 |
|
---|
| 102 | _Alternatively, you could use [safe-buffer](https://www.npmjs.com/package/safe-buffer) — it also
|
---|
| 103 | provides a polyfill, but takes a different approach which has
|
---|
| 104 | [it's drawbacks](https://github.com/chalker/safer-buffer#why-not-safe-buffer). It will allow you
|
---|
| 105 | to also use the older `new Buffer()` API in your code, though — but that's arguably a benefit, as
|
---|
| 106 | it is problematic, can cause issues in your code, and will start emitting runtime deprecation
|
---|
| 107 | warnings starting with Node.js 10._
|
---|
| 108 |
|
---|
| 109 | Note that in either case, it is important that you also remove all calls to the old Buffer
|
---|
| 110 | API manually — just throwing in `safe-buffer` doesn't fix the problem by itself, it just provides
|
---|
| 111 | a polyfill for the new API. I have seen people doing that mistake.
|
---|
| 112 |
|
---|
| 113 | Enabling eslint rule [no-buffer-constructor](https://eslint.org/docs/rules/no-buffer-constructor)
|
---|
| 114 | or
|
---|
| 115 | [node/no-deprecated-api](https://github.com/mysticatea/eslint-plugin-node/blob/master/docs/rules/no-deprecated-api.md)
|
---|
| 116 | is recommended.
|
---|
| 117 |
|
---|
| 118 | _Don't forget to drop the polyfill usage once you drop support for Node.js < 4.5.0._
|
---|
| 119 |
|
---|
| 120 | <a id="variant-3"></a>
|
---|
| 121 | ## Variant 3 — manual detection, with safeguards
|
---|
| 122 |
|
---|
| 123 | This is useful if you create Buffer instances in only a few places (e.g. one), or you have your own
|
---|
| 124 | wrapper around them.
|
---|
| 125 |
|
---|
| 126 | ### Buffer(0)
|
---|
| 127 |
|
---|
| 128 | This special case for creating empty buffers can be safely replaced with `Buffer.concat([])`, which
|
---|
| 129 | returns the same result all the way down to Node.js 0.8.x.
|
---|
| 130 |
|
---|
| 131 | ### Buffer(notNumber)
|
---|
| 132 |
|
---|
| 133 | Before:
|
---|
| 134 |
|
---|
| 135 | ```js
|
---|
| 136 | var buf = new Buffer(notNumber, encoding);
|
---|
| 137 | ```
|
---|
| 138 |
|
---|
| 139 | After:
|
---|
| 140 |
|
---|
| 141 | ```js
|
---|
| 142 | var buf;
|
---|
| 143 | if (Buffer.from && Buffer.from !== Uint8Array.from) {
|
---|
| 144 | buf = Buffer.from(notNumber, encoding);
|
---|
| 145 | } else {
|
---|
| 146 | if (typeof notNumber === 'number')
|
---|
| 147 | throw new Error('The "size" argument must be of type number.');
|
---|
| 148 | buf = new Buffer(notNumber, encoding);
|
---|
| 149 | }
|
---|
| 150 | ```
|
---|
| 151 |
|
---|
| 152 | `encoding` is optional.
|
---|
| 153 |
|
---|
| 154 | Note that the `typeof notNumber` before `new Buffer` is required (for cases when `notNumber` argument is not
|
---|
| 155 | hard-coded) and _is not caused by the deprecation of Buffer constructor_ — it's exactly _why_ the
|
---|
| 156 | Buffer constructor is deprecated. Ecosystem packages lacking this type-check caused numereous
|
---|
| 157 | security issues — situations when unsanitized user input could end up in the `Buffer(arg)` create
|
---|
| 158 | problems ranging from DoS to leaking sensitive information to the attacker from the process memory.
|
---|
| 159 |
|
---|
| 160 | When `notNumber` argument is hardcoded (e.g. literal `"abc"` or `[0,1,2]`), the `typeof` check can
|
---|
| 161 | be omitted.
|
---|
| 162 |
|
---|
| 163 | Also note that using TypeScript does not fix this problem for you — when libs written in
|
---|
| 164 | `TypeScript` are used from JS, or when user input ends up there — it behaves exactly as pure JS, as
|
---|
| 165 | all type checks are translation-time only and are not present in the actual JS code which TS
|
---|
| 166 | compiles to.
|
---|
| 167 |
|
---|
| 168 | ### Buffer(number)
|
---|
| 169 |
|
---|
| 170 | For Node.js 0.10.x (and below) support:
|
---|
| 171 |
|
---|
| 172 | ```js
|
---|
| 173 | var buf;
|
---|
| 174 | if (Buffer.alloc) {
|
---|
| 175 | buf = Buffer.alloc(number);
|
---|
| 176 | } else {
|
---|
| 177 | buf = new Buffer(number);
|
---|
| 178 | buf.fill(0);
|
---|
| 179 | }
|
---|
| 180 | ```
|
---|
| 181 |
|
---|
| 182 | Otherwise (Node.js ≥ 0.12.x):
|
---|
| 183 |
|
---|
| 184 | ```js
|
---|
| 185 | const buf = Buffer.alloc ? Buffer.alloc(number) : new Buffer(number).fill(0);
|
---|
| 186 | ```
|
---|
| 187 |
|
---|
| 188 | ## Regarding Buffer.allocUnsafe
|
---|
| 189 |
|
---|
| 190 | Be extra cautious when using `Buffer.allocUnsafe`:
|
---|
| 191 | * Don't use it if you don't have a good reason to
|
---|
| 192 | * e.g. you probably won't ever see a performance difference for small buffers, in fact, those
|
---|
| 193 | might be even faster with `Buffer.alloc()`,
|
---|
| 194 | * if your code is not in the hot code path — you also probably won't notice a difference,
|
---|
| 195 | * keep in mind that zero-filling minimizes the potential risks.
|
---|
| 196 | * If you use it, make sure that you never return the buffer in a partially-filled state,
|
---|
| 197 | * if you are writing to it sequentially — always truncate it to the actuall written length
|
---|
| 198 |
|
---|
| 199 | Errors in handling buffers allocated with `Buffer.allocUnsafe` could result in various issues,
|
---|
| 200 | ranged from undefined behaviour of your code to sensitive data (user input, passwords, certs)
|
---|
| 201 | leaking to the remote attacker.
|
---|
| 202 |
|
---|
| 203 | _Note that the same applies to `new Buffer` usage without zero-filling, depending on the Node.js
|
---|
| 204 | version (and lacking type checks also adds DoS to the list of potential problems)._
|
---|
| 205 |
|
---|
| 206 | <a id="faq"></a>
|
---|
| 207 | ## FAQ
|
---|
| 208 |
|
---|
| 209 | <a id="design-flaws"></a>
|
---|
| 210 | ### What is wrong with the `Buffer` constructor?
|
---|
| 211 |
|
---|
| 212 | The `Buffer` constructor could be used to create a buffer in many different ways:
|
---|
| 213 |
|
---|
| 214 | - `new Buffer(42)` creates a `Buffer` of 42 bytes. Before Node.js 8, this buffer contained
|
---|
| 215 | *arbitrary memory* for performance reasons, which could include anything ranging from
|
---|
| 216 | program source code to passwords and encryption keys.
|
---|
| 217 | - `new Buffer('abc')` creates a `Buffer` that contains the UTF-8-encoded version of
|
---|
| 218 | the string `'abc'`. A second argument could specify another encoding: For example,
|
---|
| 219 | `new Buffer(string, 'base64')` could be used to convert a Base64 string into the original
|
---|
| 220 | sequence of bytes that it represents.
|
---|
| 221 | - There are several other combinations of arguments.
|
---|
| 222 |
|
---|
| 223 | This meant that, in code like `var buffer = new Buffer(foo);`, *it is not possible to tell
|
---|
| 224 | what exactly the contents of the generated buffer are* without knowing the type of `foo`.
|
---|
| 225 |
|
---|
| 226 | Sometimes, the value of `foo` comes from an external source. For example, this function
|
---|
| 227 | could be exposed as a service on a web server, converting a UTF-8 string into its Base64 form:
|
---|
| 228 |
|
---|
| 229 | ```
|
---|
| 230 | function stringToBase64(req, res) {
|
---|
| 231 | // The request body should have the format of `{ string: 'foobar' }`
|
---|
| 232 | const rawBytes = new Buffer(req.body.string)
|
---|
| 233 | const encoded = rawBytes.toString('base64')
|
---|
| 234 | res.end({ encoded: encoded })
|
---|
| 235 | }
|
---|
| 236 | ```
|
---|
| 237 |
|
---|
| 238 | Note that this code does *not* validate the type of `req.body.string`:
|
---|
| 239 |
|
---|
| 240 | - `req.body.string` is expected to be a string. If this is the case, all goes well.
|
---|
| 241 | - `req.body.string` is controlled by the client that sends the request.
|
---|
| 242 | - If `req.body.string` is the *number* `50`, the `rawBytes` would be 50 bytes:
|
---|
| 243 | - Before Node.js 8, the content would be uninitialized
|
---|
| 244 | - After Node.js 8, the content would be `50` bytes with the value `0`
|
---|
| 245 |
|
---|
| 246 | Because of the missing type check, an attacker could intentionally send a number
|
---|
| 247 | as part of the request. Using this, they can either:
|
---|
| 248 |
|
---|
| 249 | - Read uninitialized memory. This **will** leak passwords, encryption keys and other
|
---|
| 250 | kinds of sensitive information. (Information leak)
|
---|
| 251 | - Force the program to allocate a large amount of memory. For example, when specifying
|
---|
| 252 | `500000000` as the input value, each request will allocate 500MB of memory.
|
---|
| 253 | This can be used to either exhaust the memory available of a program completely
|
---|
| 254 | and make it crash, or slow it down significantly. (Denial of Service)
|
---|
| 255 |
|
---|
| 256 | Both of these scenarios are considered serious security issues in a real-world
|
---|
| 257 | web server context.
|
---|
| 258 |
|
---|
| 259 | when using `Buffer.from(req.body.string)` instead, passing a number will always
|
---|
| 260 | throw an exception instead, giving a controlled behaviour that can always be
|
---|
| 261 | handled by the program.
|
---|
| 262 |
|
---|
| 263 | <a id="ecosystem-usage"></a>
|
---|
| 264 | ### The `Buffer()` constructor has been deprecated for a while. Is this really an issue?
|
---|
| 265 |
|
---|
| 266 | Surveys of code in the `npm` ecosystem have shown that the `Buffer()` constructor is still
|
---|
| 267 | widely used. This includes new code, and overall usage of such code has actually been
|
---|
| 268 | *increasing*.
|
---|