From 7d8edecbd6f425a064ca05b3ae7e55a815e0aaea Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Mon, 4 Mar 2019 17:17:05 +0100 Subject: [PATCH] buffer: do not affect memory after target for utf16 write 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: https://github.com/nodejs/node/issues/26422 PR-URL: https://github.com/nodejs/node/pull/26432 Fixes: https://github.com/nodejs/node/issues/26422 Reviewed-By: Richard Lau Reviewed-By: Ruben Bridgewater Reviewed-By: Minwoo Jung --- src/string_bytes.cc | 11 ++++++----- test/parallel/test-buffer-write.js | 8 ++++++++ 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/src/string_bytes.cc b/src/string_bytes.cc index 041868cb1e7426..131a0333be22dc 100644 --- a/src/string_bytes.cc +++ b/src/string_bytes.cc @@ -287,18 +287,19 @@ size_t StringBytes::WriteUCS2(Isolate* isolate, CHECK_EQ(reinterpret_cast(aligned_dst) % sizeof(*dst), 0); // Write all but the last char + max_chars = std::min(max_chars, static_cast(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); diff --git a/test/parallel/test-buffer-write.js b/test/parallel/test-buffer-write.js index c0c5e9c88fbc6c..a0b86844d4dbe0 100644 --- a/test/parallel/test-buffer-write.js +++ b/test/parallel/test-buffer-write.js @@ -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]); +}