Projekt

Obecné

Profil

Stáhnout (12.5 KB) Statistiky
| Větev: | Revize:
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*.
(2-2/7)