nodejs / node

Node.js JavaScript runtime ✨🐢🚀✨
https://nodejs.org
Other
107.69k stars 29.64k forks source link

Buffer.from hex creates 8Kb buffers instead of 32 bytes #41467

Closed paulmillr closed 2 years ago

paulmillr commented 2 years ago

Version

16.13.1

Platform

macOS 12.1 arm64

Subsystem

No response

What steps will reproduce the bug?

const h='0000000000000000000000000000000000000000000000000000000000000001'
const b=Buffer.from(h, 'hex')
console.log('hex in', h)
console.log('buf', b);
console.log('length', b.length);
console.log('hex out', b.toString('hex'))

console.log('.buffer');
console.log(b.buffer.byteLength);
console.log(b.buffer);

How often does it reproduce? Is there a required condition?

always

What is the expected behavior?

b.buffer should be 32 bytes just like b.length

What do you see instead?

b.buffer.byteLength is 8192 bytes. Script output:

hex in 0000000000000000000000000000000000000000000000000000000000000001
buf <Buffer 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 01>
length 32
hex out 0000000000000000000000000000000000000000000000000000000000000001
.buffer
8192
ArrayBuffer {
  [Uint8Contents]: <2f 00 00 00 00 00 00 00 2f 00 00 00 00 00 00 00 63 6f 6e 73 74 20 68 3d 27 30 30 30 30 30 30 30 30 30 30 30 30 30 30 30 30 30 30 30 30 30 30 30 30 30 30 30 30 30 30 30 30 30 30 30 30 30 30 30 30 30 30 30 30 30 30 30 30 30 30 30 30 30 30 30 30 30 30 30 30 30 30 30 31 27 0a 63 6f 6e 73 74 20 62 3d 42 ... 8092 more bytes>,
  byteLength: 8192
}

Additional information

No response

paulmillr commented 2 years ago

seems like the arraybuffer is the file itself? (same thing happens in repl)

mscdex commented 2 years ago

Buffers are created from a pool for performance reasons, so this is expected.

paulmillr commented 2 years ago

@mscdex Sorry, how is this expected? I have been using new Uint8Array(buffer.buffer) to convert node buffers to u8 arrays like everyone recommends. Now, I cannot use it, because there are special undocumented cases?

Your official documentation also has it: https://nodejs.org/api/buffer.html#buffers-and-typedarrays

mscdex commented 2 years ago

I have been using new Uint8Array(buffer.buffer) to convert node buffers to u8 arrays like everyone recommends.

I'm not sure where you're seeing such recommendations, but Buffers have been instances of Uint8Array for a very long time now, so there should be no need to convert anything.

Your official documentation also has it

The 3 uses of .buffer in the documentation you linked to are either referring to an explicit typed array's (and not a Buffer's) backing ArrayBuffer (which does not come from an ArrayBuffer pool) or are using .buffer in conjunction with offsets and lengths. All of those uses are perfectly fine.

paulmillr commented 2 years ago

Okay, i've searched through the docs and somewhere inside there is the clause that says Buffer.from shares global memory pool.

Still, this seems like a terrible security practice to a guy who does cryptography. Basically, private keys could easily leak through innocent code like Buffer.from(privKeyHex, "hex").

I am also using U8A pools to speed-up hash functions, but these are always private, nulled after write, and cannot be used by the outside modules. Just don't understand why the buffer pool is public/global.

there should be no need to convert anything.

Yes, there is: if you want your functions to always return Uint8Arrays, because browsers don't have Buffers e.g combining node crypto & webcrypto.

paulmillr commented 2 years ago

For the record, there are 800+ public code cases on GH that use Buffer.from(privateKey). Imagine how private repositories look like and what other secrets are stored there (symmetric keys, etc)

https://github.com/search?q=Buffer.from%28privateKey&type=code

DerekNonGeneric commented 2 years ago

@paulmillr, if you anticipate having malicious objects interacting w/ globals (either directly or indirectly), it is the app developer's responsibility to implement defensive programming practices for such situations themselves. Even though Node.js core APIs do have abuse protections in place, ultimately it assumes that all code being run is "trusted" by default.