Skip to content

Commit 953b4f5

Browse files
committed
src: fix TextDecoder large-input and error paths
ConverterObject::Decode() sized its ICU target buffer as the input length, or the pending byte count when flushing if that is larger, times min_char_size(), times 2. min_char_size() is the minimum number of bytes per character, so multiplying by it inflates the bound instead of tightening it: for UTF-16 (min_char_size() == 2) a 256 MiB input requested 2^30 UChars, which fails ucnv_toUnicode()'s internal targetLimit validation before any input is examined, and the failure was then reported as ERR_ENCODING_INVALID_ENCODED_DATA. Bound the buffer by 2 * (input length + pending bytes) / min_char_size instead: each character consumes at least min_char_size bytes and emits at most one surrogate pair, and bytes carried over from previous chunks complete a character in this one. The request is also clamped to ucnv_toUnicode()'s target-range validation limit of 0x3fffffff UChars, which loses nothing since larger results cannot fit in a V8 string anyway. This decodes every input whose result fits in a V8 string. Also return after a failed StringBytes::Encode() instead of falling through, so the exception it scheduled (such as ERR_STRING_TOO_LONG for results beyond the string limit) is no longer masked by ERR_ENCODING_INVALID_ENCODED_DATA. The `2 *` factor dates to 98ec909, which restored the effective capacity that an earlier targetLimit arithmetic bug had provided by accident. The min_char_size() multiplier itself is older, from ed21cb1. Fixes: #47645 Refs: #41026 Refs: #61559 Signed-off-by: Yusufhan Saçak <yusufhansacak@icloud.com>
1 parent 6cff903 commit 953b4f5

3 files changed

Lines changed: 166 additions & 12 deletions

File tree

‎src/node_i18n.cc‎

Lines changed: 19 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,7 @@
6666
#include <unicode/utypes.h>
6767
#include <unicode/uvernum.h>
6868
#include <unicode/uversion.h>
69+
#include <algorithm>
6970
#include "nbytes.h"
7071

7172
#ifdef NODE_HAVE_SMALL_ICU
@@ -446,18 +447,22 @@ void ConverterObject::Decode(const FunctionCallbackInfo<Value>& args) {
446447

447448
UBool flush = (flags & CONVERTER_FLAGS_FLUSH) == CONVERTER_FLAGS_FLUSH;
448449

449-
// When flushing the final chunk, the limit is the maximum
450-
// of either the input buffer length or the number of pending
451-
// characters times the min char size, multiplied by 2 as unicode may
452-
// take up to 2 UChars to encode a character
453-
size_t limit = 2 * converter->min_char_size() *
454-
(!flush ?
455-
input.length() :
456-
std::max(
457-
input.length(),
458-
static_cast<size_t>(
459-
ucnv_toUCountPending(converter->conv(), &status))));
450+
// ucnv_toUnicode() rejects target ranges larger than this in its
451+
// argument validation. See deps/icu-small/source/common/ucnv.cpp.
452+
constexpr size_t kMaxTargetUChars = 0x3fffffff;
453+
454+
// Each character consumes at least min_char_size() bytes and produces at
455+
// most 2 UChars (a surrogate pair). Count the bytes the converter is
456+
// still holding from previous chunks too: they belong to a character
457+
// whose remaining bytes may arrive in this chunk. Clamping to the ICU
458+
// cap loses nothing: any result that can become a V8 string needs at
459+
// most String::kMaxLength UChars, well under the cap.
460+
int32_t pending = ucnv_toUCountPending(converter->conv(), &status);
460461
status = U_ZERO_ERROR;
462+
size_t limit = std::min(
463+
2 * (input.length() + (pending > 0 ? static_cast<size_t>(pending) : 0)) /
464+
converter->min_char_size(),
465+
kMaxTargetUChars);
461466

462467
if (limit > 0)
463468
result.AllocateSufficientStorage(limit);
@@ -519,8 +524,10 @@ void ConverterObject::Decode(const FunctionCallbackInfo<Value>& args) {
519524
if (StringBytes::Encode(env->isolate(), value, length, UCS2)
520525
.ToLocal(&ret)) {
521526
args.GetReturnValue().Set(ret);
522-
return;
523527
}
528+
// If Encode() failed, it has already scheduled an exception; do not
529+
// replace it with ERR_ENCODING_INVALID_ENCODED_DATA below.
530+
return;
524531
}
525532

526533
node::THROW_ERR_ENCODING_INVALID_ENCODED_DATA(
Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,68 @@
1+
'use strict';
2+
const common = require('../common');
3+
4+
// Input large enough that the old 4x target bound exceeded ICU's
5+
// 0x3fffffff UChar limit; also needs more than a 32-bit heap.
6+
common.skipIf32Bits();
7+
8+
if (!common.hasIntl)
9+
common.skip('missing Intl');
10+
11+
// Peak RSS is around 1.6 GiB: the input, the ICU target buffer, and two
12+
// result strings.
13+
if (require('os').totalmem() < 8 * 2 ** 30)
14+
common.skip('less than 8 GiB of total memory');
15+
16+
const assert = require('assert');
17+
18+
const size = 2 ** 27;
19+
20+
let input;
21+
22+
try {
23+
input = Buffer.allocUnsafe(size * 2);
24+
} catch (e) {
25+
if (
26+
e.code === 'ERR_MEMORY_ALLOCATION_FAILED' ||
27+
/Array buffer allocation failed/.test(e.message)
28+
) {
29+
common.skip('insufficient space for Buffer.allocUnsafe');
30+
}
31+
32+
throw e;
33+
}
34+
35+
// Non-uniform repeating pattern of A, a U+1F600 surrogate pair and 中,
36+
// written as explicit little-endian bytes so the input is identical on
37+
// big-endian hosts. Corrupted or misplaced output cannot match it.
38+
input.fill(Buffer.from([0x41, 0x00, 0x3D, 0xD8, 0x00, 0xDE, 0x2D, 0x4E]));
39+
40+
const decoder = new TextDecoder('utf-16le');
41+
42+
// 2 ** 27 UTF-16 code units used to fail with
43+
// ERR_ENCODING_INVALID_ENCODED_DATA because the target buffer request
44+
// exceeded ICU's internal targetLimit validation.
45+
// Refs: https://github.com/nodejs/node/issues/47645
46+
const result = decoder.decode(input);
47+
assert.strictEqual(result.length, size);
48+
assert.strictEqual(result[0], 'A');
49+
assert.strictEqual(result[1], '\uD83D');
50+
assert.strictEqual(result[2], '\uDE00');
51+
assert.strictEqual(result[size / 2], 'A');
52+
assert.strictEqual(result[size - 1], '中');
53+
54+
// Guard against over-correction: one code unit below the failure boundary
55+
// decodes at HEAD too and must keep doing so. The truncation removes the
56+
// trailing 中, so it does not split a surrogate pair.
57+
assert.strictEqual(decoder.decode(input.subarray(0, size * 2 - 2)).length,
58+
size - 1);
59+
60+
// Streaming with an odd byte split lands mid-code-unit, so one byte stays
61+
// pending in the converter across the chunk boundary. The full content is
62+
// compared against the non-streaming result, so any corruption at the
63+
// boundary fails the test.
64+
const split = 2 ** 26 + 1;
65+
const streamed = decoder.decode(input.subarray(0, split), { stream: true }) +
66+
decoder.decode(input.subarray(split));
67+
assert.strictEqual(streamed.length, result.length);
68+
assert.strictEqual(streamed, result);
Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,79 @@
1+
'use strict';
2+
const common = require('../common');
3+
4+
// The working set is around 3 GiB, far beyond a 32-bit heap.
5+
common.skipIf32Bits();
6+
7+
if (!common.hasIntl)
8+
common.skip('missing Intl');
9+
10+
// Peak RSS is around 3 GiB: a 1 GiB input, a 2 GiB ICU target buffer, and
11+
// a transient 1 GiB StringBytes copy.
12+
if (require('os').totalmem() < 8 * 2 ** 30)
13+
common.skip('less than 8 GiB of total memory');
14+
15+
const assert = require('assert');
16+
const kStringMaxLength = require('buffer').constants.MAX_STRING_LENGTH;
17+
18+
function allocOrSkip(bytes) {
19+
try {
20+
return Buffer.allocUnsafe(bytes);
21+
} catch (e) {
22+
if (
23+
e.code === 'ERR_MEMORY_ALLOCATION_FAILED' ||
24+
/Array buffer allocation failed/.test(e.message)
25+
) {
26+
common.skip('insufficient space for Buffer.allocUnsafe');
27+
}
28+
29+
throw e;
30+
}
31+
}
32+
33+
function assertThrowsTooLong(fn) {
34+
assert.throws(fn, (e) => {
35+
// Constrained machines can fail the 1 GiB copy StringBytes makes while
36+
// building the string, before the length limit is reached.
37+
if (e.code === 'ERR_MEMORY_ALLOCATION_FAILED')
38+
common.skip('insufficient memory for the StringBytes copy');
39+
40+
assert.strictEqual(e.code, 'ERR_STRING_TOO_LONG');
41+
return true;
42+
});
43+
}
44+
45+
{
46+
// One UTF-16 code unit beyond the maximum string length: the decode
47+
// completes inside ICU but the resulting kStringMaxLength + 1 characters
48+
// cannot be materialised as a string, which must surface as
49+
// ERR_STRING_TOO_LONG rather than ERR_ENCODING_INVALID_ENCODED_DATA.
50+
// The ICU target buffer request is 2 * (size / 2) = size UChars, which
51+
// must stay <= 0x3fffffff (ICU's targetLimit cap) for the conversion to
52+
// run at all; size = 2 * kStringMaxLength + 2 = 1073741778 satisfies
53+
// that.
54+
const size = 2 * kStringMaxLength + 2;
55+
const input = allocOrSkip(size);
56+
input.fill(0x20);
57+
assertThrowsTooLong(() => new TextDecoder('utf-16le').decode(input));
58+
}
59+
60+
{
61+
// Same limit through a min_char_size() == 1 encoding: pure-ASCII input
62+
// of kStringMaxLength + 1 bytes decodes to kStringMaxLength + 1
63+
// characters. The 2x target bound is clamped to ICU's cap, so only the
64+
// output length decides the outcome. gb18030 needs full-icu; skip the
65+
// case silently on small-icu builds (the utf-16le case above ran).
66+
let decoder;
67+
try {
68+
decoder = new TextDecoder('gb18030');
69+
} catch (e) {
70+
if (e.code !== 'ERR_ENCODING_NOT_SUPPORTED')
71+
throw e;
72+
}
73+
74+
if (decoder !== undefined) {
75+
const input = allocOrSkip(kStringMaxLength + 1);
76+
input.fill(0x41);
77+
assertThrowsTooLong(() => decoder.decode(input));
78+
}
79+
}

0 commit comments

Comments
 (0)