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: #26422

PR-URL: #26432
Fixes: #26422
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Minwoo Jung <minwoo@nodesource.com>
  • Loading branch information
addaleax authored and BethGriggs committed Apr 16, 2019
1 parent 44f6260 commit ff647fd
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 @@ -286,18 +286,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 ff647fd

Please sign in to comment.