Skip to content

src: make buffers 2**32 proof in node_buffers.cc #31591

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 3 commits into from
Closed

src: make buffers 2**32 proof in node_buffers.cc #31591

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
a60a104
src: change Fill() to use ParseArrayIndex()
DavenportEmma Jan 30, 2020
52b6882
test: remove buffer overflow test
DavenportEmma Jan 31, 2020
33a26bc
fixup! test: remove buffer overflow test
Trott Feb 2, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 5 additions & 4 deletions src/node_buffer.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -590,10 +590,11 @@ void Fill(const FunctionCallbackInfo<Value>& args) {
THROW_AND_RETURN_UNLESS_BUFFER(env, args[0]);
SPREAD_BUFFER_ARG(args[0], ts_obj);

uint32_t start;
if (!args[2]->Uint32Value(ctx).To(&start)) return;
uint32_t end;
if (!args[3]->Uint32Value(ctx).To(&end)) return;
size_t start;
THROW_AND_RETURN_IF_OOB(ParseArrayIndex(env, args[2], 0, &start));
size_t end;
THROW_AND_RETURN_IF_OOB(ParseArrayIndex(env, args[3], 0, &end));
Copy link
Member

@bnoordhuis bnoordhuis Feb 1, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this is the only place that needs updating (a quick scan of node_buffer.cc suggests it is but can you double check?) then you can change the Refs: in your commit log to Fixes:.


size_t fill_length = end - start;
Local<String> str_obj;
size_t str_length;
Expand Down
20 changes: 12 additions & 8 deletions test/parallel/test-buffer-fill.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -325,10 +325,12 @@ Buffer.alloc(8, '');
assert.strictEqual(buf.toString(), 'էէէէէ');
}

// Testing process.binding. Make sure "start" is properly checked for -1 wrap
// around.
assert.strictEqual(
internalBinding('buffer').fill(Buffer.alloc(1), 1, -1, 0, 1), -2);
// Testing process.binding. Make sure "start" is properly checked for range
// errors.
assert.throws(
() => { internalBinding('buffer').fill(Buffer.alloc(1), 1, -1, 0, 1); },
{ code: 'ERR_OUT_OF_RANGE' }
);

// Make sure "end" is properly checked, even if it's magically mangled using
// Symbol.toPrimitive.
Expand All @@ -347,10 +349,12 @@ assert.strictEqual(
});
}

// Testing process.binding. Make sure "end" is properly checked for -1 wrap
// around.
assert.strictEqual(
internalBinding('buffer').fill(Buffer.alloc(1), 1, 1, -2, 1), -2);
// Testing process.binding. Make sure "end" is properly checked for range
// errors.
assert.throws(
() => { internalBinding('buffer').fill(Buffer.alloc(1), 1, 1, -2, 1); },
{ code: 'ERR_OUT_OF_RANGE' }
);

// Test that bypassing 'length' won't cause an abort.
assert.throws(() => {
Expand Down