Skip to content

Commit fc14d81

Browse files
committed
buffer: avoid overrun on UCS-2 string write
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: nodejs-private/security#203 PR-URL: nodejs-private/node-private#138
1 parent 3139897 commit fc14d81

File tree

2 files changed

+26
-1
lines changed

2 files changed

+26
-1
lines changed

src/string_bytes.cc

+5-1
Original file line numberDiff line numberDiff line change
@@ -309,7 +309,11 @@ size_t StringBytes::WriteUCS2(char* buf,
309309
size_t* chars_written) {
310310
uint16_t* const dst = reinterpret_cast<uint16_t*>(buf);
311311

312-
size_t max_chars = (buflen / sizeof(*dst));
312+
size_t max_chars = buflen / sizeof(*dst);
313+
if (max_chars == 0) {
314+
return 0;
315+
}
316+
313317
size_t nchars;
314318
size_t alignment = reinterpret_cast<uintptr_t>(dst) % sizeof(*dst);
315319
if (alignment == 0) {

test/parallel/test-buffer-write.js

+21
Original file line numberDiff line numberDiff line change
@@ -62,3 +62,24 @@ for (let i = 1; i < 10; i++) {
6262
assert.ok(!Buffer.isEncoding(encoding));
6363
assert.throws(() => Buffer.alloc(9).write('foo', encoding), error);
6464
}
65+
66+
// UCS-2 overflow CVE-2018-12115
67+
for (let i = 1; i < 4; i++) {
68+
// Allocate two Buffers sequentially off the pool. Run more than once in case
69+
// we hit the end of the pool and don't get sequential allocations
70+
const x = Buffer.allocUnsafe(4).fill(0);
71+
const y = Buffer.allocUnsafe(4).fill(1);
72+
// Should not write anything, pos 3 doesn't have enough room for a 16-bit char
73+
assert.strictEqual(x.write('ыыыыыы', 3, 'ucs2'), 0);
74+
// CVE-2018-12115 experienced via buffer overrun to next block in the pool
75+
assert.strictEqual(Buffer.compare(y, Buffer.alloc(4, 1)), 0);
76+
}
77+
78+
// Should not write any data when there is no space for 16-bit chars
79+
const z = Buffer.alloc(4, 0);
80+
assert.strictEqual(z.write('\u0001', 3, 'ucs2'), 0);
81+
assert.strictEqual(Buffer.compare(z, Buffer.alloc(4, 0)), 0);
82+
83+
// Large overrun could corrupt the process
84+
assert.strictEqual(Buffer.alloc(4)
85+
.write('ыыыыыы'.repeat(100), 3, 'utf16le'), 0);

0 commit comments

Comments
 (0)