Skip to content

Commit

Permalink
buffer: do not affect memory after target for utf16 write
Browse files Browse the repository at this point in the history
Do not write one character too much before shifting the whole result
to the left when using UTF16-LE, possibly overwriting already-used
memory while doing so.

Fixes: nodejs#26422

PR-URL: nodejs#26432
Fixes: nodejs#26422
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Minwoo Jung <[email protected]>
  • Loading branch information
addaleax authored and BridgeAR committed Mar 12, 2019
1 parent 1351f6e commit 7d8edec
Show file tree
Hide file tree
Showing 2 changed files with 14 additions and 5 deletions.
11 changes: 6 additions & 5 deletions src/string_bytes.cc
Original file line number Diff line number Diff line change
Expand Up @@ -287,18 +287,19 @@ size_t StringBytes::WriteUCS2(Isolate* isolate,
CHECK_EQ(reinterpret_cast<uintptr_t>(aligned_dst) % sizeof(*dst), 0);

// Write all but the last char
max_chars = std::min(max_chars, static_cast<size_t>(str->Length()));
if (max_chars == 0) return 0;
nchars = str->Write(isolate, aligned_dst, 0, max_chars - 1, flags);
CHECK_EQ(nchars, max_chars - 1);

// Shift everything to unaligned-left
memmove(dst, aligned_dst, nchars * sizeof(*dst));

// One more char to be written
uint16_t last;
if (nchars == max_chars - 1 &&
str->Write(isolate, &last, nchars, 1, flags) != 0) {
memcpy(buf + nchars * sizeof(*dst), &last, sizeof(last));
nchars++;
}
CHECK_EQ(str->Write(isolate, &last, nchars, 1, flags), 1);
memcpy(buf + nchars * sizeof(*dst), &last, sizeof(last));
nchars++;

*chars_written = nchars;
return nchars * sizeof(*dst);
Expand Down
8 changes: 8 additions & 0 deletions test/parallel/test-buffer-write.js
Original file line number Diff line number Diff line change
Expand Up @@ -91,3 +91,11 @@ assert.strictEqual(Buffer.compare(z, Buffer.alloc(4, 0)), 0);
// Large overrun could corrupt the process
assert.strictEqual(Buffer.alloc(4)
.write('ыыыыыы'.repeat(100), 3, 'utf16le'), 0);

{
// .write() does not affect the byte after the written-to slice of the Buffer.
// Refs: https://github.com/nodejs/node/issues/26422
const buf = Buffer.alloc(8);
assert.strictEqual(buf.write('ыы', 1, 'utf16le'), 4);
assert.deepStrictEqual([...buf], [0, 0x4b, 0x04, 0x4b, 0x04, 0, 0, 0]);
}

0 comments on commit 7d8edec

Please sign in to comment.