| [9af201e] | 1 | # safe-buffer [![travis][travis-image]][travis-url] [![npm][npm-image]][npm-url] [![downloads][downloads-image]][downloads-url] [![javascript style guide][standard-image]][standard-url]
|
|---|
| 2 |
|
|---|
| 3 | [travis-image]: https://img.shields.io/travis/feross/safe-buffer/master.svg
|
|---|
| 4 | [travis-url]: https://travis-ci.org/feross/safe-buffer
|
|---|
| 5 | [npm-image]: https://img.shields.io/npm/v/safe-buffer.svg
|
|---|
| 6 | [npm-url]: https://npmjs.org/package/safe-buffer
|
|---|
| 7 | [downloads-image]: https://img.shields.io/npm/dm/safe-buffer.svg
|
|---|
| 8 | [downloads-url]: https://npmjs.org/package/safe-buffer
|
|---|
| 9 | [standard-image]: https://img.shields.io/badge/code_style-standard-brightgreen.svg
|
|---|
| 10 | [standard-url]: https://standardjs.com
|
|---|
| 11 |
|
|---|
| 12 | #### Safer Node.js Buffer API
|
|---|
| 13 |
|
|---|
| 14 | **Use the new Node.js Buffer APIs (`Buffer.from`, `Buffer.alloc`,
|
|---|
| 15 | `Buffer.allocUnsafe`, `Buffer.allocUnsafeSlow`) in all versions of Node.js.**
|
|---|
| 16 |
|
|---|
| 17 | **Uses the built-in implementation when available.**
|
|---|
| 18 |
|
|---|
| 19 | ## install
|
|---|
| 20 |
|
|---|
| 21 | ```
|
|---|
| 22 | npm install safe-buffer
|
|---|
| 23 | ```
|
|---|
| 24 |
|
|---|
| 25 | ## usage
|
|---|
| 26 |
|
|---|
| 27 | The goal of this package is to provide a safe replacement for the node.js `Buffer`.
|
|---|
| 28 |
|
|---|
| 29 | It's a drop-in replacement for `Buffer`. You can use it by adding one `require` line to
|
|---|
| 30 | the top of your node.js modules:
|
|---|
| 31 |
|
|---|
| 32 | ```js
|
|---|
| 33 | var Buffer = require('safe-buffer').Buffer
|
|---|
| 34 |
|
|---|
| 35 | // Existing buffer code will continue to work without issues:
|
|---|
| 36 |
|
|---|
| 37 | new Buffer('hey', 'utf8')
|
|---|
| 38 | new Buffer([1, 2, 3], 'utf8')
|
|---|
| 39 | new Buffer(obj)
|
|---|
| 40 | new Buffer(16) // create an uninitialized buffer (potentially unsafe)
|
|---|
| 41 |
|
|---|
| 42 | // But you can use these new explicit APIs to make clear what you want:
|
|---|
| 43 |
|
|---|
| 44 | Buffer.from('hey', 'utf8') // convert from many types to a Buffer
|
|---|
| 45 | Buffer.alloc(16) // create a zero-filled buffer (safe)
|
|---|
| 46 | Buffer.allocUnsafe(16) // create an uninitialized buffer (potentially unsafe)
|
|---|
| 47 | ```
|
|---|
| 48 |
|
|---|
| 49 | ## api
|
|---|
| 50 |
|
|---|
| 51 | ### Class Method: Buffer.from(array)
|
|---|
| 52 | <!-- YAML
|
|---|
| 53 | added: v3.0.0
|
|---|
| 54 | -->
|
|---|
| 55 |
|
|---|
| 56 | * `array` {Array}
|
|---|
| 57 |
|
|---|
| 58 | Allocates a new `Buffer` using an `array` of octets.
|
|---|
| 59 |
|
|---|
| 60 | ```js
|
|---|
| 61 | const buf = Buffer.from([0x62,0x75,0x66,0x66,0x65,0x72]);
|
|---|
| 62 | // creates a new Buffer containing ASCII bytes
|
|---|
| 63 | // ['b','u','f','f','e','r']
|
|---|
| 64 | ```
|
|---|
| 65 |
|
|---|
| 66 | A `TypeError` will be thrown if `array` is not an `Array`.
|
|---|
| 67 |
|
|---|
| 68 | ### Class Method: Buffer.from(arrayBuffer[, byteOffset[, length]])
|
|---|
| 69 | <!-- YAML
|
|---|
| 70 | added: v5.10.0
|
|---|
| 71 | -->
|
|---|
| 72 |
|
|---|
| 73 | * `arrayBuffer` {ArrayBuffer} The `.buffer` property of a `TypedArray` or
|
|---|
| 74 | a `new ArrayBuffer()`
|
|---|
| 75 | * `byteOffset` {Number} Default: `0`
|
|---|
| 76 | * `length` {Number} Default: `arrayBuffer.length - byteOffset`
|
|---|
| 77 |
|
|---|
| 78 | When passed a reference to the `.buffer` property of a `TypedArray` instance,
|
|---|
| 79 | the newly created `Buffer` will share the same allocated memory as the
|
|---|
| 80 | TypedArray.
|
|---|
| 81 |
|
|---|
| 82 | ```js
|
|---|
| 83 | const arr = new Uint16Array(2);
|
|---|
| 84 | arr[0] = 5000;
|
|---|
| 85 | arr[1] = 4000;
|
|---|
| 86 |
|
|---|
| 87 | const buf = Buffer.from(arr.buffer); // shares the memory with arr;
|
|---|
| 88 |
|
|---|
| 89 | console.log(buf);
|
|---|
| 90 | // Prints: <Buffer 88 13 a0 0f>
|
|---|
| 91 |
|
|---|
| 92 | // changing the TypedArray changes the Buffer also
|
|---|
| 93 | arr[1] = 6000;
|
|---|
| 94 |
|
|---|
| 95 | console.log(buf);
|
|---|
| 96 | // Prints: <Buffer 88 13 70 17>
|
|---|
| 97 | ```
|
|---|
| 98 |
|
|---|
| 99 | The optional `byteOffset` and `length` arguments specify a memory range within
|
|---|
| 100 | the `arrayBuffer` that will be shared by the `Buffer`.
|
|---|
| 101 |
|
|---|
| 102 | ```js
|
|---|
| 103 | const ab = new ArrayBuffer(10);
|
|---|
| 104 | const buf = Buffer.from(ab, 0, 2);
|
|---|
| 105 | console.log(buf.length);
|
|---|
| 106 | // Prints: 2
|
|---|
| 107 | ```
|
|---|
| 108 |
|
|---|
| 109 | A `TypeError` will be thrown if `arrayBuffer` is not an `ArrayBuffer`.
|
|---|
| 110 |
|
|---|
| 111 | ### Class Method: Buffer.from(buffer)
|
|---|
| 112 | <!-- YAML
|
|---|
| 113 | added: v3.0.0
|
|---|
| 114 | -->
|
|---|
| 115 |
|
|---|
| 116 | * `buffer` {Buffer}
|
|---|
| 117 |
|
|---|
| 118 | Copies the passed `buffer` data onto a new `Buffer` instance.
|
|---|
| 119 |
|
|---|
| 120 | ```js
|
|---|
| 121 | const buf1 = Buffer.from('buffer');
|
|---|
| 122 | const buf2 = Buffer.from(buf1);
|
|---|
| 123 |
|
|---|
| 124 | buf1[0] = 0x61;
|
|---|
| 125 | console.log(buf1.toString());
|
|---|
| 126 | // 'auffer'
|
|---|
| 127 | console.log(buf2.toString());
|
|---|
| 128 | // 'buffer' (copy is not changed)
|
|---|
| 129 | ```
|
|---|
| 130 |
|
|---|
| 131 | A `TypeError` will be thrown if `buffer` is not a `Buffer`.
|
|---|
| 132 |
|
|---|
| 133 | ### Class Method: Buffer.from(str[, encoding])
|
|---|
| 134 | <!-- YAML
|
|---|
| 135 | added: v5.10.0
|
|---|
| 136 | -->
|
|---|
| 137 |
|
|---|
| 138 | * `str` {String} String to encode.
|
|---|
| 139 | * `encoding` {String} Encoding to use, Default: `'utf8'`
|
|---|
| 140 |
|
|---|
| 141 | Creates a new `Buffer` containing the given JavaScript string `str`. If
|
|---|
| 142 | provided, the `encoding` parameter identifies the character encoding.
|
|---|
| 143 | If not provided, `encoding` defaults to `'utf8'`.
|
|---|
| 144 |
|
|---|
| 145 | ```js
|
|---|
| 146 | const buf1 = Buffer.from('this is a tést');
|
|---|
| 147 | console.log(buf1.toString());
|
|---|
| 148 | // prints: this is a tést
|
|---|
| 149 | console.log(buf1.toString('ascii'));
|
|---|
| 150 | // prints: this is a tC)st
|
|---|
| 151 |
|
|---|
| 152 | const buf2 = Buffer.from('7468697320697320612074c3a97374', 'hex');
|
|---|
| 153 | console.log(buf2.toString());
|
|---|
| 154 | // prints: this is a tést
|
|---|
| 155 | ```
|
|---|
| 156 |
|
|---|
| 157 | A `TypeError` will be thrown if `str` is not a string.
|
|---|
| 158 |
|
|---|
| 159 | ### Class Method: Buffer.alloc(size[, fill[, encoding]])
|
|---|
| 160 | <!-- YAML
|
|---|
| 161 | added: v5.10.0
|
|---|
| 162 | -->
|
|---|
| 163 |
|
|---|
| 164 | * `size` {Number}
|
|---|
| 165 | * `fill` {Value} Default: `undefined`
|
|---|
| 166 | * `encoding` {String} Default: `utf8`
|
|---|
| 167 |
|
|---|
| 168 | Allocates a new `Buffer` of `size` bytes. If `fill` is `undefined`, the
|
|---|
| 169 | `Buffer` will be *zero-filled*.
|
|---|
| 170 |
|
|---|
| 171 | ```js
|
|---|
| 172 | const buf = Buffer.alloc(5);
|
|---|
| 173 | console.log(buf);
|
|---|
| 174 | // <Buffer 00 00 00 00 00>
|
|---|
| 175 | ```
|
|---|
| 176 |
|
|---|
| 177 | The `size` must be less than or equal to the value of
|
|---|
| 178 | `require('buffer').kMaxLength` (on 64-bit architectures, `kMaxLength` is
|
|---|
| 179 | `(2^31)-1`). Otherwise, a [`RangeError`][] is thrown. A zero-length Buffer will
|
|---|
| 180 | be created if a `size` less than or equal to 0 is specified.
|
|---|
| 181 |
|
|---|
| 182 | If `fill` is specified, the allocated `Buffer` will be initialized by calling
|
|---|
| 183 | `buf.fill(fill)`. See [`buf.fill()`][] for more information.
|
|---|
| 184 |
|
|---|
| 185 | ```js
|
|---|
| 186 | const buf = Buffer.alloc(5, 'a');
|
|---|
| 187 | console.log(buf);
|
|---|
| 188 | // <Buffer 61 61 61 61 61>
|
|---|
| 189 | ```
|
|---|
| 190 |
|
|---|
| 191 | If both `fill` and `encoding` are specified, the allocated `Buffer` will be
|
|---|
| 192 | initialized by calling `buf.fill(fill, encoding)`. For example:
|
|---|
| 193 |
|
|---|
| 194 | ```js
|
|---|
| 195 | const buf = Buffer.alloc(11, 'aGVsbG8gd29ybGQ=', 'base64');
|
|---|
| 196 | console.log(buf);
|
|---|
| 197 | // <Buffer 68 65 6c 6c 6f 20 77 6f 72 6c 64>
|
|---|
| 198 | ```
|
|---|
| 199 |
|
|---|
| 200 | Calling `Buffer.alloc(size)` can be significantly slower than the alternative
|
|---|
| 201 | `Buffer.allocUnsafe(size)` but ensures that the newly created `Buffer` instance
|
|---|
| 202 | contents will *never contain sensitive data*.
|
|---|
| 203 |
|
|---|
| 204 | A `TypeError` will be thrown if `size` is not a number.
|
|---|
| 205 |
|
|---|
| 206 | ### Class Method: Buffer.allocUnsafe(size)
|
|---|
| 207 | <!-- YAML
|
|---|
| 208 | added: v5.10.0
|
|---|
| 209 | -->
|
|---|
| 210 |
|
|---|
| 211 | * `size` {Number}
|
|---|
| 212 |
|
|---|
| 213 | Allocates a new *non-zero-filled* `Buffer` of `size` bytes. The `size` must
|
|---|
| 214 | be less than or equal to the value of `require('buffer').kMaxLength` (on 64-bit
|
|---|
| 215 | architectures, `kMaxLength` is `(2^31)-1`). Otherwise, a [`RangeError`][] is
|
|---|
| 216 | thrown. A zero-length Buffer will be created if a `size` less than or equal to
|
|---|
| 217 | 0 is specified.
|
|---|
| 218 |
|
|---|
| 219 | The underlying memory for `Buffer` instances created in this way is *not
|
|---|
| 220 | initialized*. The contents of the newly created `Buffer` are unknown and
|
|---|
| 221 | *may contain sensitive data*. Use [`buf.fill(0)`][] to initialize such
|
|---|
| 222 | `Buffer` instances to zeroes.
|
|---|
| 223 |
|
|---|
| 224 | ```js
|
|---|
| 225 | const buf = Buffer.allocUnsafe(5);
|
|---|
| 226 | console.log(buf);
|
|---|
| 227 | // <Buffer 78 e0 82 02 01>
|
|---|
| 228 | // (octets will be different, every time)
|
|---|
| 229 | buf.fill(0);
|
|---|
| 230 | console.log(buf);
|
|---|
| 231 | // <Buffer 00 00 00 00 00>
|
|---|
| 232 | ```
|
|---|
| 233 |
|
|---|
| 234 | A `TypeError` will be thrown if `size` is not a number.
|
|---|
| 235 |
|
|---|
| 236 | Note that the `Buffer` module pre-allocates an internal `Buffer` instance of
|
|---|
| 237 | size `Buffer.poolSize` that is used as a pool for the fast allocation of new
|
|---|
| 238 | `Buffer` instances created using `Buffer.allocUnsafe(size)` (and the deprecated
|
|---|
| 239 | `new Buffer(size)` constructor) only when `size` is less than or equal to
|
|---|
| 240 | `Buffer.poolSize >> 1` (floor of `Buffer.poolSize` divided by two). The default
|
|---|
| 241 | value of `Buffer.poolSize` is `8192` but can be modified.
|
|---|
| 242 |
|
|---|
| 243 | Use of this pre-allocated internal memory pool is a key difference between
|
|---|
| 244 | calling `Buffer.alloc(size, fill)` vs. `Buffer.allocUnsafe(size).fill(fill)`.
|
|---|
| 245 | Specifically, `Buffer.alloc(size, fill)` will *never* use the internal Buffer
|
|---|
| 246 | pool, while `Buffer.allocUnsafe(size).fill(fill)` *will* use the internal
|
|---|
| 247 | Buffer pool if `size` is less than or equal to half `Buffer.poolSize`. The
|
|---|
| 248 | difference is subtle but can be important when an application requires the
|
|---|
| 249 | additional performance that `Buffer.allocUnsafe(size)` provides.
|
|---|
| 250 |
|
|---|
| 251 | ### Class Method: Buffer.allocUnsafeSlow(size)
|
|---|
| 252 | <!-- YAML
|
|---|
| 253 | added: v5.10.0
|
|---|
| 254 | -->
|
|---|
| 255 |
|
|---|
| 256 | * `size` {Number}
|
|---|
| 257 |
|
|---|
| 258 | Allocates a new *non-zero-filled* and non-pooled `Buffer` of `size` bytes. The
|
|---|
| 259 | `size` must be less than or equal to the value of
|
|---|
| 260 | `require('buffer').kMaxLength` (on 64-bit architectures, `kMaxLength` is
|
|---|
| 261 | `(2^31)-1`). Otherwise, a [`RangeError`][] is thrown. A zero-length Buffer will
|
|---|
| 262 | be created if a `size` less than or equal to 0 is specified.
|
|---|
| 263 |
|
|---|
| 264 | The underlying memory for `Buffer` instances created in this way is *not
|
|---|
| 265 | initialized*. The contents of the newly created `Buffer` are unknown and
|
|---|
| 266 | *may contain sensitive data*. Use [`buf.fill(0)`][] to initialize such
|
|---|
| 267 | `Buffer` instances to zeroes.
|
|---|
| 268 |
|
|---|
| 269 | When using `Buffer.allocUnsafe()` to allocate new `Buffer` instances,
|
|---|
| 270 | allocations under 4KB are, by default, sliced from a single pre-allocated
|
|---|
| 271 | `Buffer`. This allows applications to avoid the garbage collection overhead of
|
|---|
| 272 | creating many individually allocated Buffers. This approach improves both
|
|---|
| 273 | performance and memory usage by eliminating the need to track and cleanup as
|
|---|
| 274 | many `Persistent` objects.
|
|---|
| 275 |
|
|---|
| 276 | However, in the case where a developer may need to retain a small chunk of
|
|---|
| 277 | memory from a pool for an indeterminate amount of time, it may be appropriate
|
|---|
| 278 | to create an un-pooled Buffer instance using `Buffer.allocUnsafeSlow()` then
|
|---|
| 279 | copy out the relevant bits.
|
|---|
| 280 |
|
|---|
| 281 | ```js
|
|---|
| 282 | // need to keep around a few small chunks of memory
|
|---|
| 283 | const store = [];
|
|---|
| 284 |
|
|---|
| 285 | socket.on('readable', () => {
|
|---|
| 286 | const data = socket.read();
|
|---|
| 287 | // allocate for retained data
|
|---|
| 288 | const sb = Buffer.allocUnsafeSlow(10);
|
|---|
| 289 | // copy the data into the new allocation
|
|---|
| 290 | data.copy(sb, 0, 0, 10);
|
|---|
| 291 | store.push(sb);
|
|---|
| 292 | });
|
|---|
| 293 | ```
|
|---|
| 294 |
|
|---|
| 295 | Use of `Buffer.allocUnsafeSlow()` should be used only as a last resort *after*
|
|---|
| 296 | a developer has observed undue memory retention in their applications.
|
|---|
| 297 |
|
|---|
| 298 | A `TypeError` will be thrown if `size` is not a number.
|
|---|
| 299 |
|
|---|
| 300 | ### All the Rest
|
|---|
| 301 |
|
|---|
| 302 | The rest of the `Buffer` API is exactly the same as in node.js.
|
|---|
| 303 | [See the docs](https://nodejs.org/api/buffer.html).
|
|---|
| 304 |
|
|---|
| 305 |
|
|---|
| 306 | ## Related links
|
|---|
| 307 |
|
|---|
| 308 | - [Node.js issue: Buffer(number) is unsafe](https://github.com/nodejs/node/issues/4660)
|
|---|
| 309 | - [Node.js Enhancement Proposal: Buffer.from/Buffer.alloc/Buffer.zalloc/Buffer() soft-deprecate](https://github.com/nodejs/node-eps/pull/4)
|
|---|
| 310 |
|
|---|
| 311 | ## Why is `Buffer` unsafe?
|
|---|
| 312 |
|
|---|
| 313 | Today, the node.js `Buffer` constructor is overloaded to handle many different argument
|
|---|
| 314 | types like `String`, `Array`, `Object`, `TypedArrayView` (`Uint8Array`, etc.),
|
|---|
| 315 | `ArrayBuffer`, and also `Number`.
|
|---|
| 316 |
|
|---|
| 317 | The API is optimized for convenience: you can throw any type at it, and it will try to do
|
|---|
| 318 | what you want.
|
|---|
| 319 |
|
|---|
| 320 | Because the Buffer constructor is so powerful, you often see code like this:
|
|---|
| 321 |
|
|---|
| 322 | ```js
|
|---|
| 323 | // Convert UTF-8 strings to hex
|
|---|
| 324 | function toHex (str) {
|
|---|
| 325 | return new Buffer(str).toString('hex')
|
|---|
| 326 | }
|
|---|
| 327 | ```
|
|---|
| 328 |
|
|---|
| 329 | ***But what happens if `toHex` is called with a `Number` argument?***
|
|---|
| 330 |
|
|---|
| 331 | ### Remote Memory Disclosure
|
|---|
| 332 |
|
|---|
| 333 | If an attacker can make your program call the `Buffer` constructor with a `Number`
|
|---|
| 334 | argument, then they can make it allocate uninitialized memory from the node.js process.
|
|---|
| 335 | This could potentially disclose TLS private keys, user data, or database passwords.
|
|---|
| 336 |
|
|---|
| 337 | When the `Buffer` constructor is passed a `Number` argument, it returns an
|
|---|
| 338 | **UNINITIALIZED** block of memory of the specified `size`. When you create a `Buffer` like
|
|---|
| 339 | this, you **MUST** overwrite the contents before returning it to the user.
|
|---|
| 340 |
|
|---|
| 341 | From the [node.js docs](https://nodejs.org/api/buffer.html#buffer_new_buffer_size):
|
|---|
| 342 |
|
|---|
| 343 | > `new Buffer(size)`
|
|---|
| 344 | >
|
|---|
| 345 | > - `size` Number
|
|---|
| 346 | >
|
|---|
| 347 | > The underlying memory for `Buffer` instances created in this way is not initialized.
|
|---|
| 348 | > **The contents of a newly created `Buffer` are unknown and could contain sensitive
|
|---|
| 349 | > data.** Use `buf.fill(0)` to initialize a Buffer to zeroes.
|
|---|
| 350 |
|
|---|
| 351 | (Emphasis our own.)
|
|---|
| 352 |
|
|---|
| 353 | Whenever the programmer intended to create an uninitialized `Buffer` you often see code
|
|---|
| 354 | like this:
|
|---|
| 355 |
|
|---|
| 356 | ```js
|
|---|
| 357 | var buf = new Buffer(16)
|
|---|
| 358 |
|
|---|
| 359 | // Immediately overwrite the uninitialized buffer with data from another buffer
|
|---|
| 360 | for (var i = 0; i < buf.length; i++) {
|
|---|
| 361 | buf[i] = otherBuf[i]
|
|---|
| 362 | }
|
|---|
| 363 | ```
|
|---|
| 364 |
|
|---|
| 365 |
|
|---|
| 366 | ### Would this ever be a problem in real code?
|
|---|
| 367 |
|
|---|
| 368 | Yes. It's surprisingly common to forget to check the type of your variables in a
|
|---|
| 369 | dynamically-typed language like JavaScript.
|
|---|
| 370 |
|
|---|
| 371 | Usually the consequences of assuming the wrong type is that your program crashes with an
|
|---|
| 372 | uncaught exception. But the failure mode for forgetting to check the type of arguments to
|
|---|
| 373 | the `Buffer` constructor is more catastrophic.
|
|---|
| 374 |
|
|---|
| 375 | Here's an example of a vulnerable service that takes a JSON payload and converts it to
|
|---|
| 376 | hex:
|
|---|
| 377 |
|
|---|
| 378 | ```js
|
|---|
| 379 | // Take a JSON payload {str: "some string"} and convert it to hex
|
|---|
| 380 | var server = http.createServer(function (req, res) {
|
|---|
| 381 | var data = ''
|
|---|
| 382 | req.setEncoding('utf8')
|
|---|
| 383 | req.on('data', function (chunk) {
|
|---|
| 384 | data += chunk
|
|---|
| 385 | })
|
|---|
| 386 | req.on('end', function () {
|
|---|
| 387 | var body = JSON.parse(data)
|
|---|
| 388 | res.end(new Buffer(body.str).toString('hex'))
|
|---|
| 389 | })
|
|---|
| 390 | })
|
|---|
| 391 |
|
|---|
| 392 | server.listen(8080)
|
|---|
| 393 | ```
|
|---|
| 394 |
|
|---|
| 395 | In this example, an http client just has to send:
|
|---|
| 396 |
|
|---|
| 397 | ```json
|
|---|
| 398 | {
|
|---|
| 399 | "str": 1000
|
|---|
| 400 | }
|
|---|
| 401 | ```
|
|---|
| 402 |
|
|---|
| 403 | and it will get back 1,000 bytes of uninitialized memory from the server.
|
|---|
| 404 |
|
|---|
| 405 | This is a very serious bug. It's similar in severity to the
|
|---|
| 406 | [the Heartbleed bug](http://heartbleed.com/) that allowed disclosure of OpenSSL process
|
|---|
| 407 | memory by remote attackers.
|
|---|
| 408 |
|
|---|
| 409 |
|
|---|
| 410 | ### Which real-world packages were vulnerable?
|
|---|
| 411 |
|
|---|
| 412 | #### [`bittorrent-dht`](https://www.npmjs.com/package/bittorrent-dht)
|
|---|
| 413 |
|
|---|
| 414 | [Mathias Buus](https://github.com/mafintosh) and I
|
|---|
| 415 | ([Feross Aboukhadijeh](http://feross.org/)) found this issue in one of our own packages,
|
|---|
| 416 | [`bittorrent-dht`](https://www.npmjs.com/package/bittorrent-dht). The bug would allow
|
|---|
| 417 | anyone on the internet to send a series of messages to a user of `bittorrent-dht` and get
|
|---|
| 418 | them to reveal 20 bytes at a time of uninitialized memory from the node.js process.
|
|---|
| 419 |
|
|---|
| 420 | Here's
|
|---|
| 421 | [the commit](https://github.com/feross/bittorrent-dht/commit/6c7da04025d5633699800a99ec3fbadf70ad35b8)
|
|---|
| 422 | that fixed it. We released a new fixed version, created a
|
|---|
| 423 | [Node Security Project disclosure](https://nodesecurity.io/advisories/68), and deprecated all
|
|---|
| 424 | vulnerable versions on npm so users will get a warning to upgrade to a newer version.
|
|---|
| 425 |
|
|---|
| 426 | #### [`ws`](https://www.npmjs.com/package/ws)
|
|---|
| 427 |
|
|---|
| 428 | That got us wondering if there were other vulnerable packages. Sure enough, within a short
|
|---|
| 429 | period of time, we found the same issue in [`ws`](https://www.npmjs.com/package/ws), the
|
|---|
| 430 | most popular WebSocket implementation in node.js.
|
|---|
| 431 |
|
|---|
| 432 | If certain APIs were called with `Number` parameters instead of `String` or `Buffer` as
|
|---|
| 433 | expected, then uninitialized server memory would be disclosed to the remote peer.
|
|---|
| 434 |
|
|---|
| 435 | These were the vulnerable methods:
|
|---|
| 436 |
|
|---|
| 437 | ```js
|
|---|
| 438 | socket.send(number)
|
|---|
| 439 | socket.ping(number)
|
|---|
| 440 | socket.pong(number)
|
|---|
| 441 | ```
|
|---|
| 442 |
|
|---|
| 443 | Here's a vulnerable socket server with some echo functionality:
|
|---|
| 444 |
|
|---|
| 445 | ```js
|
|---|
| 446 | server.on('connection', function (socket) {
|
|---|
| 447 | socket.on('message', function (message) {
|
|---|
| 448 | message = JSON.parse(message)
|
|---|
| 449 | if (message.type === 'echo') {
|
|---|
| 450 | socket.send(message.data) // send back the user's message
|
|---|
| 451 | }
|
|---|
| 452 | })
|
|---|
| 453 | })
|
|---|
| 454 | ```
|
|---|
| 455 |
|
|---|
| 456 | `socket.send(number)` called on the server, will disclose server memory.
|
|---|
| 457 |
|
|---|
| 458 | Here's [the release](https://github.com/websockets/ws/releases/tag/1.0.1) where the issue
|
|---|
| 459 | was fixed, with a more detailed explanation. Props to
|
|---|
| 460 | [Arnout Kazemier](https://github.com/3rd-Eden) for the quick fix. Here's the
|
|---|
| 461 | [Node Security Project disclosure](https://nodesecurity.io/advisories/67).
|
|---|
| 462 |
|
|---|
| 463 |
|
|---|
| 464 | ### What's the solution?
|
|---|
| 465 |
|
|---|
| 466 | It's important that node.js offers a fast way to get memory otherwise performance-critical
|
|---|
| 467 | applications would needlessly get a lot slower.
|
|---|
| 468 |
|
|---|
| 469 | But we need a better way to *signal our intent* as programmers. **When we want
|
|---|
| 470 | uninitialized memory, we should request it explicitly.**
|
|---|
| 471 |
|
|---|
| 472 | Sensitive functionality should not be packed into a developer-friendly API that loosely
|
|---|
| 473 | accepts many different types. This type of API encourages the lazy practice of passing
|
|---|
| 474 | variables in without checking the type very carefully.
|
|---|
| 475 |
|
|---|
| 476 | #### A new API: `Buffer.allocUnsafe(number)`
|
|---|
| 477 |
|
|---|
| 478 | The functionality of creating buffers with uninitialized memory should be part of another
|
|---|
| 479 | API. We propose `Buffer.allocUnsafe(number)`. This way, it's not part of an API that
|
|---|
| 480 | frequently gets user input of all sorts of different types passed into it.
|
|---|
| 481 |
|
|---|
| 482 | ```js
|
|---|
| 483 | var buf = Buffer.allocUnsafe(16) // careful, uninitialized memory!
|
|---|
| 484 |
|
|---|
| 485 | // Immediately overwrite the uninitialized buffer with data from another buffer
|
|---|
| 486 | for (var i = 0; i < buf.length; i++) {
|
|---|
| 487 | buf[i] = otherBuf[i]
|
|---|
| 488 | }
|
|---|
| 489 | ```
|
|---|
| 490 |
|
|---|
| 491 |
|
|---|
| 492 | ### How do we fix node.js core?
|
|---|
| 493 |
|
|---|
| 494 | We sent [a PR to node.js core](https://github.com/nodejs/node/pull/4514) (merged as
|
|---|
| 495 | `semver-major`) which defends against one case:
|
|---|
| 496 |
|
|---|
| 497 | ```js
|
|---|
| 498 | var str = 16
|
|---|
| 499 | new Buffer(str, 'utf8')
|
|---|
| 500 | ```
|
|---|
| 501 |
|
|---|
| 502 | In this situation, it's implied that the programmer intended the first argument to be a
|
|---|
| 503 | string, since they passed an encoding as a second argument. Today, node.js will allocate
|
|---|
| 504 | uninitialized memory in the case of `new Buffer(number, encoding)`, which is probably not
|
|---|
| 505 | what the programmer intended.
|
|---|
| 506 |
|
|---|
| 507 | But this is only a partial solution, since if the programmer does `new Buffer(variable)`
|
|---|
| 508 | (without an `encoding` parameter) there's no way to know what they intended. If `variable`
|
|---|
| 509 | is sometimes a number, then uninitialized memory will sometimes be returned.
|
|---|
| 510 |
|
|---|
| 511 | ### What's the real long-term fix?
|
|---|
| 512 |
|
|---|
| 513 | We could deprecate and remove `new Buffer(number)` and use `Buffer.allocUnsafe(number)` when
|
|---|
| 514 | we need uninitialized memory. But that would break 1000s of packages.
|
|---|
| 515 |
|
|---|
| 516 | ~~We believe the best solution is to:~~
|
|---|
| 517 |
|
|---|
| 518 | ~~1. Change `new Buffer(number)` to return safe, zeroed-out memory~~
|
|---|
| 519 |
|
|---|
| 520 | ~~2. Create a new API for creating uninitialized Buffers. We propose: `Buffer.allocUnsafe(number)`~~
|
|---|
| 521 |
|
|---|
| 522 | #### Update
|
|---|
| 523 |
|
|---|
| 524 | We now support adding three new APIs:
|
|---|
| 525 |
|
|---|
| 526 | - `Buffer.from(value)` - convert from any type to a buffer
|
|---|
| 527 | - `Buffer.alloc(size)` - create a zero-filled buffer
|
|---|
| 528 | - `Buffer.allocUnsafe(size)` - create an uninitialized buffer with given size
|
|---|
| 529 |
|
|---|
| 530 | This solves the core problem that affected `ws` and `bittorrent-dht` which is
|
|---|
| 531 | `Buffer(variable)` getting tricked into taking a number argument.
|
|---|
| 532 |
|
|---|
| 533 | This way, existing code continues working and the impact on the npm ecosystem will be
|
|---|
| 534 | minimal. Over time, npm maintainers can migrate performance-critical code to use
|
|---|
| 535 | `Buffer.allocUnsafe(number)` instead of `new Buffer(number)`.
|
|---|
| 536 |
|
|---|
| 537 |
|
|---|
| 538 | ### Conclusion
|
|---|
| 539 |
|
|---|
| 540 | We think there's a serious design issue with the `Buffer` API as it exists today. It
|
|---|
| 541 | promotes insecure software by putting high-risk functionality into a convenient API
|
|---|
| 542 | with friendly "developer ergonomics".
|
|---|
| 543 |
|
|---|
| 544 | This wasn't merely a theoretical exercise because we found the issue in some of the
|
|---|
| 545 | most popular npm packages.
|
|---|
| 546 |
|
|---|
| 547 | Fortunately, there's an easy fix that can be applied today. Use `safe-buffer` in place of
|
|---|
| 548 | `buffer`.
|
|---|
| 549 |
|
|---|
| 550 | ```js
|
|---|
| 551 | var Buffer = require('safe-buffer').Buffer
|
|---|
| 552 | ```
|
|---|
| 553 |
|
|---|
| 554 | Eventually, we hope that node.js core can switch to this new, safer behavior. We believe
|
|---|
| 555 | the impact on the ecosystem would be minimal since it's not a breaking change.
|
|---|
| 556 | Well-maintained, popular packages would be updated to use `Buffer.alloc` quickly, while
|
|---|
| 557 | older, insecure packages would magically become safe from this attack vector.
|
|---|
| 558 |
|
|---|
| 559 |
|
|---|
| 560 | ## links
|
|---|
| 561 |
|
|---|
| 562 | - [Node.js PR: buffer: throw if both length and enc are passed](https://github.com/nodejs/node/pull/4514)
|
|---|
| 563 | - [Node Security Project disclosure for `ws`](https://nodesecurity.io/advisories/67)
|
|---|
| 564 | - [Node Security Project disclosure for`bittorrent-dht`](https://nodesecurity.io/advisories/68)
|
|---|
| 565 |
|
|---|
| 566 |
|
|---|
| 567 | ## credit
|
|---|
| 568 |
|
|---|
| 569 | The original issues in `bittorrent-dht`
|
|---|
| 570 | ([disclosure](https://nodesecurity.io/advisories/68)) and
|
|---|
| 571 | `ws` ([disclosure](https://nodesecurity.io/advisories/67)) were discovered by
|
|---|
| 572 | [Mathias Buus](https://github.com/mafintosh) and
|
|---|
| 573 | [Feross Aboukhadijeh](http://feross.org/).
|
|---|
| 574 |
|
|---|
| 575 | Thanks to [Adam Baldwin](https://github.com/evilpacket) for helping disclose these issues
|
|---|
| 576 | and for his work running the [Node Security Project](https://nodesecurity.io/).
|
|---|
| 577 |
|
|---|
| 578 | Thanks to [John Hiesey](https://github.com/jhiesey) for proofreading this README and
|
|---|
| 579 | auditing the code.
|
|---|
| 580 |
|
|---|
| 581 |
|
|---|
| 582 | ## license
|
|---|
| 583 |
|
|---|
| 584 | MIT. Copyright (C) [Feross Aboukhadijeh](http://feross.org)
|
|---|