Skip to content

Commit

Permalink
buffer: mark pool ArrayBuffer as untransferable
Browse files Browse the repository at this point in the history
This removes a footgun in which users could attempt to transfer the
pooled ArrayBuffer underlying a regular `Buffer`, which would lead to
all `Buffer`s that share the same pool being rendered unusable as well,
and potentially break creation of new pooled `Buffer`s.

This disables this kind of transfer.

Refs: #32752

PR-URL: #32759
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Gus Caplan <[email protected]>
Reviewed-By: James M Snell <[email protected]>
  • Loading branch information
addaleax authored and targos committed Apr 28, 2020
1 parent c26637b commit b53193a
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 1 deletion.
5 changes: 4 additions & 1 deletion lib/buffer.js
Original file line number Diff line number Diff line change
Expand Up @@ -59,11 +59,13 @@ const {
zeroFill: bindingZeroFill
} = internalBinding('buffer');
const {
arraybuffer_untransferable_private_symbol,
getOwnNonIndexProperties,
propertyFilter: {
ALL_PROPERTIES,
ONLY_ENUMERABLE
}
},
setHiddenValue,
} = internalBinding('util');
const {
customInspectSymbol,
Expand Down Expand Up @@ -153,6 +155,7 @@ function createUnsafeBuffer(size) {
function createPool() {
poolSize = Buffer.poolSize;
allocPool = createUnsafeBuffer(poolSize).buffer;
setHiddenValue(allocPool, arraybuffer_untransferable_private_symbol, true);
poolOffset = 0;
}
createPool();
Expand Down
20 changes: 20 additions & 0 deletions test/parallel/test-buffer-pool-untransferable.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
'use strict';
require('../common');
const assert = require('assert');
const { MessageChannel } = require('worker_threads');

// Make sure that the pools used by the Buffer implementation are not
// transferable.
// Refs: https://github.com/nodejs/node/issues/32752

const a = Buffer.from('hello world');
const b = Buffer.from('hello world');
assert.strictEqual(a.buffer, b.buffer);
const length = a.length;

const { port1 } = new MessageChannel();
port1.postMessage(a, [ a.buffer ]);

// Verify that the pool ArrayBuffer has not actually been transfered:
assert.strictEqual(a.buffer, b.buffer);
assert.strictEqual(a.length, length);

0 comments on commit b53193a

Please sign in to comment.