From 0052926476474a28747004321e37f1dd4969d250 Mon Sep 17 00:00:00 2001 From: Rod Vagg 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.js | 21 +++++++++++++++++++++ 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/src/string_bytes.cc b/src/string_bytes.cc index 882ca6e3e89bd3..b54b7a2b36cdac 100644 --- a/src/string_bytes.cc +++ b/src/string_bytes.cc @@ -226,7 +226,11 @@ size_t StringBytes::WriteUCS2(char* buf, size_t* chars_written) { uint16_t* const dst = reinterpret_cast(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(dst) % sizeof(*dst); if (alignment == 0) { diff --git a/test/parallel/test-buffer.js b/test/parallel/test-buffer.js index 6b1848c48bb9cb..36cf8373cee6bd 100644 --- a/test/parallel/test-buffer.js +++ b/test/parallel/test-buffer.js @@ -1506,3 +1506,24 @@ assert.strictEqual(SlowBuffer.prototype.offset, undefined); // Check pool offset after that by trying to write string into the pool. assert.doesNotThrow(() => Buffer.from('abc')); } + +// 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);