|
 |
8c1749 |
From 0052926476474a28747004321e37f1dd4969d250 Mon Sep 17 00:00:00 2001
|
|
|
8c1749 |
From: Rod Vagg <rod@vagg.org>
|
|
|
8c1749 |
Date: Tue, 14 Aug 2018 20:18:06 +1000
|
|
|
8c1749 |
Subject: [PATCH] buffer: avoid overrun on UCS-2 string write
|
|
|
8c1749 |
MIME-Version: 1.0
|
|
|
8c1749 |
Content-Type: text/plain; charset=UTF-8
|
|
|
8c1749 |
Content-Transfer-Encoding: 8bit
|
|
|
8c1749 |
|
|
|
8c1749 |
CVE-2018-12115
|
|
|
8c1749 |
Discovered by ChALkeR - Сковорода Никита Андреевич
|
|
|
8c1749 |
Fix by Anna Henningsen
|
|
|
8c1749 |
|
|
|
8c1749 |
Writing to the second-to-last byte with UCS-2 encoding will cause a -1
|
|
|
8c1749 |
length to be send to String::Write(), writing all of the provided Buffer
|
|
|
8c1749 |
from that point and beyond.
|
|
|
8c1749 |
|
|
|
8c1749 |
Fixes: https://github.com/nodejs-private/security/issues/203
|
|
|
8c1749 |
PR-URL: https://github.com/nodejs-private/node-private/pull/138
|
|
|
8c1749 |
---
|
|
|
8c1749 |
src/string_bytes.cc | 6 +++++-
|
|
|
8c1749 |
test/parallel/test-buffer.js | 21 +++++++++++++++++++++
|
|
|
8c1749 |
2 files changed, 26 insertions(+), 1 deletion(-)
|
|
|
8c1749 |
|
|
|
8c1749 |
diff --git a/src/string_bytes.cc b/src/string_bytes.cc
|
|
|
8c1749 |
index 882ca6e3e89..b54b7a2b36c 100644
|
|
|
8c1749 |
--- a/src/string_bytes.cc
|
|
|
8c1749 |
+++ b/src/string_bytes.cc
|
|
|
8c1749 |
@@ -226,7 +226,11 @@ size_t StringBytes::WriteUCS2(char* buf,
|
|
|
8c1749 |
size_t* chars_written) {
|
|
|
8c1749 |
uint16_t* const dst = reinterpret_cast<uint16_t*>(buf);
|
|
|
8c1749 |
|
|
|
8c1749 |
- size_t max_chars = (buflen / sizeof(*dst));
|
|
|
8c1749 |
+ size_t max_chars = buflen / sizeof(*dst);
|
|
|
8c1749 |
+ if (max_chars == 0) {
|
|
|
8c1749 |
+ return 0;
|
|
|
8c1749 |
+ }
|
|
|
8c1749 |
+
|
|
|
8c1749 |
size_t nchars;
|
|
|
8c1749 |
size_t alignment = reinterpret_cast<uintptr_t>(dst) % sizeof(*dst);
|
|
|
8c1749 |
if (alignment == 0) {
|
|
|
8c1749 |
diff --git a/test/parallel/test-buffer.js b/test/parallel/test-buffer.js
|
|
|
8c1749 |
index 6b1848c48bb..36cf8373cee 100644
|
|
|
8c1749 |
--- a/test/parallel/test-buffer.js
|
|
|
8c1749 |
+++ b/test/parallel/test-buffer.js
|
|
|
8c1749 |
@@ -1506,3 +1506,24 @@ assert.strictEqual(SlowBuffer.prototype.offset, undefined);
|
|
|
8c1749 |
// Check pool offset after that by trying to write string into the pool.
|
|
|
8c1749 |
assert.doesNotThrow(() => Buffer.from('abc'));
|
|
|
8c1749 |
}
|
|
|
8c1749 |
+
|
|
|
8c1749 |
+// UCS-2 overflow CVE-2018-12115
|
|
|
8c1749 |
+for (let i = 1; i < 4; i++) {
|
|
|
8c1749 |
+ // Allocate two Buffers sequentially off the pool. Run more than once in case
|
|
|
8c1749 |
+ // we hit the end of the pool and don't get sequential allocations
|
|
|
8c1749 |
+ const x = Buffer.allocUnsafe(4).fill(0);
|
|
|
8c1749 |
+ const y = Buffer.allocUnsafe(4).fill(1);
|
|
|
8c1749 |
+ // Should not write anything, pos 3 doesn't have enough room for a 16-bit char
|
|
|
8c1749 |
+ assert.strictEqual(x.write('ыыыыыы', 3, 'ucs2'), 0);
|
|
|
8c1749 |
+ // CVE-2018-12115 experienced via buffer overrun to next block in the pool
|
|
|
8c1749 |
+ assert.strictEqual(Buffer.compare(y, Buffer.alloc(4, 1)), 0);
|
|
|
8c1749 |
+}
|
|
|
8c1749 |
+
|
|
|
8c1749 |
+// Should not write any data when there is no space for 16-bit chars
|
|
|
8c1749 |
+const z = Buffer.alloc(4, 0);
|
|
|
8c1749 |
+assert.strictEqual(z.write('\u0001', 3, 'ucs2'), 0);
|
|
|
8c1749 |
+assert.strictEqual(Buffer.compare(z, Buffer.alloc(4, 0)), 0);
|
|
|
8c1749 |
+
|
|
|
8c1749 |
+// Large overrun could corrupt the process
|
|
|
8c1749 |
+assert.strictEqual(Buffer.alloc(4)
|
|
|
8c1749 |
+ .write('ыыыыыы'.repeat(100), 3, 'utf16le'), 0);
|