Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit c33933e

Browse filesBrowse files
TimothyGuMylesBorins
TimothyGu
authored and
MylesBorins
committed
src, buffer: do not segfault on out-of-range index
Also add test cases for partial writes and invalid indices. PR-URL: #11927 Fixes: #8724 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net>
1 parent 3801790 commit c33933e
Copy full SHA for c33933e

File tree

Expand file treeCollapse file tree

2 files changed

+75
-16
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

2 files changed

+75
-16
lines changed
Open diff view settings
Collapse file

‎src/node_buffer.cc‎

Copy file name to clipboardExpand all lines: src/node_buffer.cc
+20-8Lines changed: 20 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -159,7 +159,8 @@ void CallbackInfo::WeakCallback(Isolate* isolate) {
159159
// Parse index for external array data.
160160
inline MUST_USE_RESULT bool ParseArrayIndex(Local<Value> arg,
161161
size_t def,
162-
size_t* ret) {
162+
size_t* ret,
163+
size_t needed = 0) {
163164
if (arg->IsUndefined()) {
164165
*ret = def;
165166
return true;
@@ -173,7 +174,7 @@ inline MUST_USE_RESULT bool ParseArrayIndex(Local<Value> arg,
173174
// Check that the result fits in a size_t.
174175
const uint64_t kSizeMax = static_cast<uint64_t>(static_cast<size_t>(-1));
175176
// coverity[pointless_expression]
176-
if (static_cast<uint64_t>(tmp_i) > kSizeMax)
177+
if (static_cast<uint64_t>(tmp_i) > kSizeMax - needed)
177178
return false;
178179

179180
*ret = static_cast<size_t>(tmp_i);
@@ -805,17 +806,28 @@ void WriteFloatGeneric(const FunctionCallbackInfo<Value>& args) {
805806
CHECK_NE(ts_obj_data, nullptr);
806807

807808
T val = args[1]->NumberValue(env->context()).FromMaybe(0);
808-
size_t offset = args[2]->IntegerValue(env->context()).FromMaybe(0);
809809

810810
size_t memcpy_num = sizeof(T);
811+
size_t offset;
811812

812-
if (should_assert) {
813-
THROW_AND_RETURN_IF_OOB(offset + memcpy_num >= memcpy_num);
814-
THROW_AND_RETURN_IF_OOB(offset + memcpy_num <= ts_obj_length);
813+
// If the offset is negative or larger than the size of the ArrayBuffer,
814+
// throw an error (if needed) and return directly.
815+
if (!ParseArrayIndex(args[2], 0, &offset, memcpy_num) ||
816+
offset >= ts_obj_length) {
817+
if (should_assert)
818+
THROW_AND_RETURN_IF_OOB(false);
819+
return;
815820
}
816821

817-
if (offset + memcpy_num > ts_obj_length)
818-
memcpy_num = ts_obj_length - offset;
822+
// If the offset is too large for the entire value, but small enough to fit
823+
// part of the value, throw an error and return only if should_assert is
824+
// true. Otherwise, write the part of the value that fits.
825+
if (offset + memcpy_num > ts_obj_length) {
826+
if (should_assert)
827+
THROW_AND_RETURN_IF_OOB(false);
828+
else
829+
memcpy_num = ts_obj_length - offset;
830+
}
819831

820832
union NoAlias {
821833
T val;
Collapse file

‎test/parallel/test-buffer-write-noassert.js‎

Copy file name to clipboardExpand all lines: test/parallel/test-buffer-write-noassert.js
+55-8Lines changed: 55 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -4,28 +4,33 @@ const assert = require('assert');
44

55
// testing buffer write functions
66

7+
const outOfRange = /^RangeError: (?:Index )?out of range(?: index)?$/;
8+
79
function write(funx, args, result, res) {
810
{
911
const buf = Buffer.alloc(9);
1012
assert.strictEqual(buf[funx](...args), result);
1113
assert.deepStrictEqual(buf, res);
1214
}
1315

14-
{
15-
const invalidArgs = Array.from(args);
16-
invalidArgs[1] = -1;
17-
assert.throws(
18-
() => Buffer.alloc(9)[funx](...invalidArgs),
19-
/^RangeError: (?:Index )?out of range(?: index)?$/
20-
);
21-
}
16+
writeInvalidOffset(-1);
17+
writeInvalidOffset(9);
2218

2319
{
2420
const buf2 = Buffer.alloc(9);
2521
assert.strictEqual(buf2[funx](...args, true), result);
2622
assert.deepStrictEqual(buf2, res);
2723
}
2824

25+
function writeInvalidOffset(offset) {
26+
const newArgs = Array.from(args);
27+
newArgs[1] = offset;
28+
assert.throws(() => Buffer.alloc(9)[funx](...newArgs), outOfRange);
29+
30+
const buf = Buffer.alloc(9);
31+
buf[funx](...newArgs, true);
32+
assert.deepStrictEqual(buf, Buffer.alloc(9));
33+
}
2934
}
3035

3136
write('writeInt8', [1, 0], 1, Buffer.from([1, 0, 0, 0, 0, 0, 0, 0, 0]));
@@ -46,3 +51,45 @@ write('writeDoubleBE', [1, 1], 9, Buffer.from([0, 63, 240, 0, 0, 0, 0, 0, 0]));
4651
write('writeDoubleLE', [1, 1], 9, Buffer.from([0, 0, 0, 0, 0, 0, 0, 240, 63]));
4752
write('writeFloatBE', [1, 1], 5, Buffer.from([0, 63, 128, 0, 0, 0, 0, 0, 0]));
4853
write('writeFloatLE', [1, 1], 5, Buffer.from([0, 0, 0, 128, 63, 0, 0, 0, 0]));
54+
55+
function writePartial(funx, args, result, res) {
56+
assert.throws(() => Buffer.alloc(9)[funx](...args), outOfRange);
57+
const buf = Buffer.alloc(9);
58+
assert.strictEqual(buf[funx](...args, true), result);
59+
assert.deepStrictEqual(buf, res);
60+
}
61+
62+
// Test partial writes (cases where the buffer isn't large enough to hold the
63+
// entire value, but is large enough to hold parts of it).
64+
writePartial('writeIntBE', [0x0eadbeef, 6, 4], 10,
65+
Buffer.from([0, 0, 0, 0, 0, 0, 0x0e, 0xad, 0xbe]));
66+
writePartial('writeIntLE', [0x0eadbeef, 6, 4], 10,
67+
Buffer.from([0, 0, 0, 0, 0, 0, 0xef, 0xbe, 0xad]));
68+
writePartial('writeInt16BE', [0x1234, 8], 10,
69+
Buffer.from([0, 0, 0, 0, 0, 0, 0, 0, 0x12]));
70+
writePartial('writeInt16LE', [0x1234, 8], 10,
71+
Buffer.from([0, 0, 0, 0, 0, 0, 0, 0, 0x34]));
72+
writePartial('writeInt32BE', [0x0eadbeef, 6], 10,
73+
Buffer.from([0, 0, 0, 0, 0, 0, 0x0e, 0xad, 0xbe]));
74+
writePartial('writeInt32LE', [0x0eadbeef, 6], 10,
75+
Buffer.from([0, 0, 0, 0, 0, 0, 0xef, 0xbe, 0xad]));
76+
writePartial('writeUIntBE', [0xdeadbeef, 6, 4], 10,
77+
Buffer.from([0, 0, 0, 0, 0, 0, 0xde, 0xad, 0xbe]));
78+
writePartial('writeUIntLE', [0xdeadbeef, 6, 4], 10,
79+
Buffer.from([0, 0, 0, 0, 0, 0, 0xef, 0xbe, 0xad]));
80+
writePartial('writeUInt16BE', [0x1234, 8], 10,
81+
Buffer.from([0, 0, 0, 0, 0, 0, 0, 0, 0x12]));
82+
writePartial('writeUInt16LE', [0x1234, 8], 10,
83+
Buffer.from([0, 0, 0, 0, 0, 0, 0, 0, 0x34]));
84+
writePartial('writeUInt32BE', [0xdeadbeef, 6], 10,
85+
Buffer.from([0, 0, 0, 0, 0, 0, 0xde, 0xad, 0xbe]));
86+
writePartial('writeUInt32LE', [0xdeadbeef, 6], 10,
87+
Buffer.from([0, 0, 0, 0, 0, 0, 0xef, 0xbe, 0xad]));
88+
writePartial('writeDoubleBE', [1, 2], 10,
89+
Buffer.from([0, 0, 63, 240, 0, 0, 0, 0, 0]));
90+
writePartial('writeDoubleLE', [1, 2], 10,
91+
Buffer.from([0, 0, 0, 0, 0, 0, 0, 0, 240]));
92+
writePartial('writeFloatBE', [1, 6], 10,
93+
Buffer.from([0, 0, 0, 0, 0, 0, 63, 128, 0]));
94+
writePartial('writeFloatLE', [1, 6], 10,
95+
Buffer.from([0, 0, 0, 0, 0, 0, 0, 0, 128]));

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.