From fc14d812b742cfec4e0a1ee1573c5636709c2bce Mon Sep 17 00:00:00 2001
From: Rod Vagg <rod@vagg.org>
Date: Tue, 14 Aug 2018 20:18:06 +1000
Subject: [PATCH] buffer: avoid overrun on UCS-2 string write
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

CVE-2018-12115
Discovered by ChALkeR - Сковорода Никита Андреевич
Fix by Anna Henningsen

Writing to the second-to-last byte with UCS-2 encoding will cause a -1
length to be send to String::Write(), writing all of the provided Buffer
from that point and beyond.

Fixes: https://github.com/nodejs-private/security/issues/203
PR-URL: https://github.com/nodejs-private/node-private/pull/138
---
 src/string_bytes.cc                |  6 +++++-
 test/parallel/test-buffer-write.js | 21 +++++++++++++++++++++
 2 files changed, 26 insertions(+), 1 deletion(-)

diff --git a/src/string_bytes.cc b/src/string_bytes.cc
index 3333f03386ea73..258bc72cf62a43 100644
--- a/src/string_bytes.cc
+++ b/src/string_bytes.cc
@@ -309,7 +309,11 @@ size_t StringBytes::WriteUCS2(char* buf,
                               size_t* chars_written) {
   uint16_t* const dst = reinterpret_cast<uint16_t*>(buf);
 
-  size_t max_chars = (buflen / sizeof(*dst));
+  size_t max_chars = buflen / sizeof(*dst);
+  if (max_chars == 0) {
+    return 0;
+  }
+
   size_t nchars;
   size_t alignment = reinterpret_cast<uintptr_t>(dst) % sizeof(*dst);
   if (alignment == 0) {
diff --git a/test/parallel/test-buffer-write.js b/test/parallel/test-buffer-write.js
index 34bba7b1e7c1d8..eb029c0adfaf5b 100644
--- a/test/parallel/test-buffer-write.js
+++ b/test/parallel/test-buffer-write.js
@@ -62,3 +62,24 @@ for (let i = 1; i < 10; i++) {
   assert.ok(!Buffer.isEncoding(encoding));
   assert.throws(() => Buffer.alloc(9).write('foo', encoding), error);
 }
+
+// UCS-2 overflow CVE-2018-12115
+for (let i = 1; i < 4; i++) {
+  // Allocate two Buffers sequentially off the pool. Run more than once in case
+  // we hit the end of the pool and don't get sequential allocations
+  const x = Buffer.allocUnsafe(4).fill(0);
+  const y = Buffer.allocUnsafe(4).fill(1);
+  // Should not write anything, pos 3 doesn't have enough room for a 16-bit char
+  assert.strictEqual(x.write('ыыыыыы', 3, 'ucs2'), 0);
+  // CVE-2018-12115 experienced via buffer overrun to next block in the pool
+  assert.strictEqual(Buffer.compare(y, Buffer.alloc(4, 1)), 0);
+}
+
+// Should not write any data when there is no space for 16-bit chars
+const z = Buffer.alloc(4, 0);
+assert.strictEqual(z.write('\u0001', 3, 'ucs2'), 0);
+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);