What to do about Buffer?

[sam-github]

The history of Buffer is confusing!

I said in
https://github.com/nodejs/security-wg/blob/master/meetings/2019-06-17.md that I
would look at this, and possibly write a blog post. I don't think I'll have
time for a blog post, but this summarizes my findings so far.

nodejs/node#4660

Describes how to trigger memory exposure. It also has the epic conversation
about what to do about it...

nodejs/node#8169, etc.

It seems that new Buffer() was preferred to Buffer() for reasons having to
do with inheritance (which I don't understand), as well as security? Example:

• Buffer() without a new keyword is going to be hard-deprecated soon sidorares/node-mysql2#380

I think this is because b = new Buffer(num) zero-allocated, where b = Buffer(num) did not.

nodejs/node#7152

Describes security and inheritance reasons to runtime-deprecate Buffer
constructor.

ChALkeR's Writeups

• https://github.com/ChALkeR/notes/blob/master/Lets-fix-Buffer-API.md
• https://github.com/ChALkeR/notes/blob/master/Buffer-knows-everything.md

Best overviews, but they are dated, since they don't reflect what happened
in v8.0.0. I also think they no longer reflect his opinion (or maybe what
happened wasn't exactly what he preferred?).

Note that even zero-filling Buffer(number) by default will not remove
the need in this change, because Buffer(number) could be still used to
cause a DoS in a simple way if the code is accidentally calling
Buffer(number) instead of Buffer(value).

- [REF](https://github.com/ChALkeR/notes/blob/master/Lets-fix-Buffer-API.md#second-part--replace-buffervalue-with-a-separate-method)

Only description so far about why we currently have a problem with
the deprecated constructor, since it now zero-fills in all supported Node.js
versions. The issue is that huge zeroed memory can be allocated when it was
thought by authors that only small Buffers with a copy of a string would be
allocated. This seems to me not a serious enough issue for us to force the
ecosystem off of the Buffer constructor, now that all Node.js < 8.x is EOL.

However, I'm not insensitive to the fact that 6.x is still used in deployments,
so vulnerabilities specific to it may still be relevant.

See #505 (comment)

Buffer pooling...

It turns out the presence of Buffer pooling causes the memory exposure problem
to be possible again. There isn't a lot of information about this yet, but
it is referenced in nodejs/node#28439

What next?

I've PRed a summary of what the problem with the deprecated constructor is
in nodejs/node#28825, because that information was
unreasonably hard to find.

Please review, @nodejs/security-wg!

There was discussion of a blog post in last sec-wg meeting. I'm not sure what
content is, though. I think some more discussion has to happen, perhaps in this
issue, to determine what the path(s) forward are, and see if we can come to
some consensus as to what should be done.

Discussion might also occur in nodejs/node#28439, where
ChALKeR is proposing an opt-in approach to secure Buffers.

[UlisesGascon]

Thanks @sam-github for share all the resources. I can help with the blog post, but I am not an expert on this topic. :-)

[github-actions]

This issue is stale because it has been open many days with no activity. It will be closed soon unless the stale label is removed or a comment is made.