| 1 | # safer-buffer [![travis][travis-image]][travis-url] [![npm][npm-image]][npm-url] [![javascript style guide][standard-image]][standard-url] [![Security Responsible Disclosure][secuirty-image]][secuirty-url]
|
|---|
| 2 |
|
|---|
| 3 | [travis-image]: https://travis-ci.org/ChALkeR/safer-buffer.svg?branch=master
|
|---|
| 4 | [travis-url]: https://travis-ci.org/ChALkeR/safer-buffer
|
|---|
| 5 | [npm-image]: https://img.shields.io/npm/v/safer-buffer.svg
|
|---|
| 6 | [npm-url]: https://npmjs.org/package/safer-buffer
|
|---|
| 7 | [standard-image]: https://img.shields.io/badge/code_style-standard-brightgreen.svg
|
|---|
| 8 | [standard-url]: https://standardjs.com
|
|---|
| 9 | [secuirty-image]: https://img.shields.io/badge/Security-Responsible%20Disclosure-green.svg
|
|---|
| 10 | [secuirty-url]: https://github.com/nodejs/security-wg/blob/master/processes/responsible_disclosure_template.md
|
|---|
| 11 |
|
|---|
| 12 | Modern Buffer API polyfill without footguns, working on Node.js from 0.8 to current.
|
|---|
| 13 |
|
|---|
| 14 | ## How to use?
|
|---|
| 15 |
|
|---|
| 16 | First, port all `Buffer()` and `new Buffer()` calls to `Buffer.alloc()` and `Buffer.from()` API.
|
|---|
| 17 |
|
|---|
| 18 | Then, to achieve compatibility with outdated Node.js versions (`<4.5.0` and 5.x `<5.9.0`), use
|
|---|
| 19 | `const Buffer = require('safer-buffer').Buffer` in all files where you make calls to the new
|
|---|
| 20 | Buffer API. _Use `var` instead of `const` if you need that for your Node.js version range support._
|
|---|
| 21 |
|
|---|
| 22 | Also, see the
|
|---|
| 23 | [porting Buffer](https://github.com/ChALkeR/safer-buffer/blob/master/Porting-Buffer.md) guide.
|
|---|
| 24 |
|
|---|
| 25 | ## Do I need it?
|
|---|
| 26 |
|
|---|
| 27 | Hopefully, not — dropping support for outdated Node.js versions should be fine nowdays, and that
|
|---|
| 28 | is the recommended path forward. You _do_ need to port to the `Buffer.alloc()` and `Buffer.from()`
|
|---|
| 29 | though.
|
|---|
| 30 |
|
|---|
| 31 | See the [porting guide](https://github.com/ChALkeR/safer-buffer/blob/master/Porting-Buffer.md)
|
|---|
| 32 | for a better description.
|
|---|
| 33 |
|
|---|
| 34 | ## Why not [safe-buffer](https://npmjs.com/safe-buffer)?
|
|---|
| 35 |
|
|---|
| 36 | _In short: while `safe-buffer` serves as a polyfill for the new API, it allows old API usage and
|
|---|
| 37 | itself contains footguns._
|
|---|
| 38 |
|
|---|
| 39 | `safe-buffer` could be used safely to get the new API while still keeping support for older
|
|---|
| 40 | Node.js versions (like this module), but while analyzing ecosystem usage of the old Buffer API
|
|---|
| 41 | I found out that `safe-buffer` is itself causing problems in some cases.
|
|---|
| 42 |
|
|---|
| 43 | For example, consider the following snippet:
|
|---|
| 44 |
|
|---|
| 45 | ```console
|
|---|
| 46 | $ cat example.unsafe.js
|
|---|
| 47 | console.log(Buffer(20))
|
|---|
| 48 | $ ./node-v6.13.0-linux-x64/bin/node example.unsafe.js
|
|---|
| 49 | <Buffer 0a 00 00 00 00 00 00 00 28 13 de 02 00 00 00 00 05 00 00 00>
|
|---|
| 50 | $ standard example.unsafe.js
|
|---|
| 51 | standard: Use JavaScript Standard Style (https://standardjs.com)
|
|---|
| 52 | /home/chalker/repo/safer-buffer/example.unsafe.js:2:13: 'Buffer()' was deprecated since v6. Use 'Buffer.alloc()' or 'Buffer.from()' (use 'https://www.npmjs.com/package/safe-buffer' for '<4.5.0') instead.
|
|---|
| 53 | ```
|
|---|
| 54 |
|
|---|
| 55 | This is allocates and writes to console an uninitialized chunk of memory.
|
|---|
| 56 | [standard](https://www.npmjs.com/package/standard) linter (among others) catch that and warn people
|
|---|
| 57 | to avoid using unsafe API.
|
|---|
| 58 |
|
|---|
| 59 | Let's now throw in `safe-buffer`!
|
|---|
| 60 |
|
|---|
| 61 | ```console
|
|---|
| 62 | $ cat example.safe-buffer.js
|
|---|
| 63 | const Buffer = require('safe-buffer').Buffer
|
|---|
| 64 | console.log(Buffer(20))
|
|---|
| 65 | $ standard example.safe-buffer.js
|
|---|
| 66 | $ ./node-v6.13.0-linux-x64/bin/node example.safe-buffer.js
|
|---|
| 67 | <Buffer 08 00 00 00 00 00 00 00 28 58 01 82 fe 7f 00 00 00 00 00 00>
|
|---|
| 68 | ```
|
|---|
| 69 |
|
|---|
| 70 | See the problem? Adding in `safe-buffer` _magically removes the lint warning_, but the behavior
|
|---|
| 71 | remains identiсal to what we had before, and when launched on Node.js 6.x LTS — this dumps out
|
|---|
| 72 | chunks of uninitialized memory.
|
|---|
| 73 | _And this code will still emit runtime warnings on Node.js 10.x and above._
|
|---|
| 74 |
|
|---|
| 75 | That was done by design. I first considered changing `safe-buffer`, prohibiting old API usage or
|
|---|
| 76 | emitting warnings on it, but that significantly diverges from `safe-buffer` design. After some
|
|---|
| 77 | discussion, it was decided to move my approach into a separate package, and _this is that separate
|
|---|
| 78 | package_.
|
|---|
| 79 |
|
|---|
| 80 | This footgun is not imaginary — I observed top-downloaded packages doing that kind of thing,
|
|---|
| 81 | «fixing» the lint warning by blindly including `safe-buffer` without any actual changes.
|
|---|
| 82 |
|
|---|
| 83 | Also in some cases, even if the API _was_ migrated to use of safe Buffer API — a random pull request
|
|---|
| 84 | can bring unsafe Buffer API usage back to the codebase by adding new calls — and that could go
|
|---|
| 85 | unnoticed even if you have a linter prohibiting that (becase of the reason stated above), and even
|
|---|
| 86 | pass CI. _I also observed that being done in popular packages._
|
|---|
| 87 |
|
|---|
| 88 | Some examples:
|
|---|
| 89 | * [webdriverio](https://github.com/webdriverio/webdriverio/commit/05cbd3167c12e4930f09ef7cf93b127ba4effae4#diff-124380949022817b90b622871837d56cR31)
|
|---|
| 90 | (a module with 548 759 downloads/month),
|
|---|
| 91 | * [websocket-stream](https://github.com/maxogden/websocket-stream/commit/c9312bd24d08271687d76da0fe3c83493871cf61)
|
|---|
| 92 | (218 288 d/m, fix in [maxogden/websocket-stream#142](https://github.com/maxogden/websocket-stream/pull/142)),
|
|---|
| 93 | * [node-serialport](https://github.com/node-serialport/node-serialport/commit/e8d9d2b16c664224920ce1c895199b1ce2def48c)
|
|---|
| 94 | (113 138 d/m, fix in [node-serialport/node-serialport#1510](https://github.com/node-serialport/node-serialport/pull/1510)),
|
|---|
| 95 | * [karma](https://github.com/karma-runner/karma/commit/3d94b8cf18c695104ca195334dc75ff054c74eec)
|
|---|
| 96 | (3 973 193 d/m, fix in [karma-runner/karma#2947](https://github.com/karma-runner/karma/pull/2947)),
|
|---|
| 97 | * [spdy-transport](https://github.com/spdy-http2/spdy-transport/commit/5375ac33f4a62a4f65bcfc2827447d42a5dbe8b1)
|
|---|
| 98 | (5 970 727 d/m, fix in [spdy-http2/spdy-transport#53](https://github.com/spdy-http2/spdy-transport/pull/53)).
|
|---|
| 99 | * And there are a lot more over the ecosystem.
|
|---|
| 100 |
|
|---|
| 101 | I filed a PR at
|
|---|
| 102 | [mysticatea/eslint-plugin-node#110](https://github.com/mysticatea/eslint-plugin-node/pull/110) to
|
|---|
| 103 | partially fix that (for cases when that lint rule is used), but it is a semver-major change for
|
|---|
| 104 | linter rules and presets, so it would take significant time for that to reach actual setups.
|
|---|
| 105 | _It also hasn't been released yet (2018-03-20)._
|
|---|
| 106 |
|
|---|
| 107 | Also, `safer-buffer` discourages the usage of `.allocUnsafe()`, which is often done by a mistake.
|
|---|
| 108 | It still supports it with an explicit concern barier, by placing it under
|
|---|
| 109 | `require('safer-buffer/dangereous')`.
|
|---|
| 110 |
|
|---|
| 111 | ## But isn't throwing bad?
|
|---|
| 112 |
|
|---|
| 113 | Not really. It's an error that could be noticed and fixed early, instead of causing havoc later like
|
|---|
| 114 | unguarded `new Buffer()` calls that end up receiving user input can do.
|
|---|
| 115 |
|
|---|
| 116 | This package affects only the files where `var Buffer = require('safer-buffer').Buffer` was done, so
|
|---|
| 117 | it is really simple to keep track of things and make sure that you don't mix old API usage with that.
|
|---|
| 118 | Also, CI should hint anything that you might have missed.
|
|---|
| 119 |
|
|---|
| 120 | New commits, if tested, won't land new usage of unsafe Buffer API this way.
|
|---|
| 121 | _Node.js 10.x also deals with that by printing a runtime depecation warning._
|
|---|
| 122 |
|
|---|
| 123 | ### Would it affect third-party modules?
|
|---|
| 124 |
|
|---|
| 125 | No, unless you explicitly do an awful thing like monkey-patching or overriding the built-in `Buffer`.
|
|---|
| 126 | Don't do that.
|
|---|
| 127 |
|
|---|
| 128 | ### But I don't want throwing…
|
|---|
| 129 |
|
|---|
| 130 | That is also fine!
|
|---|
| 131 |
|
|---|
| 132 | Also, it could be better in some cases when you don't comprehensive enough test coverage.
|
|---|
| 133 |
|
|---|
| 134 | In that case — just don't override `Buffer` and use
|
|---|
| 135 | `var SaferBuffer = require('safer-buffer').Buffer` instead.
|
|---|
| 136 |
|
|---|
| 137 | That way, everything using `Buffer` natively would still work, but there would be two drawbacks:
|
|---|
| 138 |
|
|---|
| 139 | * `Buffer.from`/`Buffer.alloc` won't be polyfilled — use `SaferBuffer.from` and
|
|---|
| 140 | `SaferBuffer.alloc` instead.
|
|---|
| 141 | * You are still open to accidentally using the insecure deprecated API — use a linter to catch that.
|
|---|
| 142 |
|
|---|
| 143 | Note that using a linter to catch accidential `Buffer` constructor usage in this case is strongly
|
|---|
| 144 | recommended. `Buffer` is not overriden in this usecase, so linters won't get confused.
|
|---|
| 145 |
|
|---|
| 146 | ## «Without footguns»?
|
|---|
| 147 |
|
|---|
| 148 | Well, it is still possible to do _some_ things with `Buffer` API, e.g. accessing `.buffer` property
|
|---|
| 149 | on older versions and duping things from there. You shouldn't do that in your code, probabably.
|
|---|
| 150 |
|
|---|
| 151 | The intention is to remove the most significant footguns that affect lots of packages in the
|
|---|
| 152 | ecosystem, and to do it in the proper way.
|
|---|
| 153 |
|
|---|
| 154 | Also, this package doesn't protect against security issues affecting some Node.js versions, so for
|
|---|
| 155 | usage in your own production code, it is still recommended to update to a Node.js version
|
|---|
| 156 | [supported by upstream](https://github.com/nodejs/release#release-schedule).
|
|---|
