ICU-23404 ucnv_lmb: clamp toULength before LMBCS reassembly memcpy

[evilgensec]

Summary

LMBCS toUnicode at the top of _LMBCSToUnicodeWithOffsets (icu4c/source/common/ucnv_lmb.cpp:1281-1290) reassembles a partial character from args->converter->toUBytes into the local 3-byte stack buffer LMBCS (ULMBCS_CHARSIZE_MAX = 3). It reads size_old = args->converter->toULength and copies that many bytes into LMBCS without first comparing against sizeof(LMBCS).

toULength is int8_t and elsewhere in ICU is allowed to hold up to UCNV_MAX_CHAR_LEN - 1 = 7. The LMBCS converter itself never sets it higher than 3, but the field is part of the public-ish UConverter state and survives across ucnv_toUnicode calls. If an application reuses a converter object across encodings without calling ucnv_reset() (technically API-non-compliant but encountered in the wild), or a non-LMBCS error callback leaves a longer partial sequence in toUBytes, size_old reaches the LMBCS code with a value larger than 3 and:

• uprv_memcpy(LMBCS, ...toUBytes, size_old) overflows the 3-byte stack array;
• size_new_maybe_1 = sizeof(LMBCS) - size_old is size_t, so the subtraction wraps to near-SIZE_MAX and the subsequent MIN may pick a large size_new, compounding the overflow on the second memcpy.

Fix

Add a defensive size_old > sizeof(LMBCS) check that rejects the input with U_INVALID_STATE_ERROR. Mirrors the pattern used in other ICU converters that validate their inherited partial-sequence state before consuming it.

Diff

 icu4c/source/common/ucnv_lmb.cpp | 17 ++++++++++++++---
 1 file changed, 14 insertions(+), 3 deletions(-)

Threat model

Pure caller-error invariant violation (API misuse). Defense-in-depth hardening; not a directly exploitable parser bug. The LMBCS converter does not itself produce toULength > 3, but several common usage patterns can leave that field in a stale state:

• An application reuses one UConverter* across encodings without ucnv_reset() between switches.
• A custom UConverterToUCallback writes more than 3 bytes into toUBytes for a non-LMBCS partial sequence, then the application changes the converter's algorithm.

Without this guard, both patterns smash the 3-byte stack array. With the guard the converter cleanly returns U_INVALID_STATE_ERROR.

Refs

Same defensive pattern is already present in _LMBCSFromUnicodeWithOffsets for the encode side, and other ICU converters apply equivalent toULength validation at conversion entry (e.g. ucnv_u8.cpp, ucnvmbcs.cpp).

[CLAassistant]

All committers have signed the CLA.

[Copilot]

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Adds a defensive check in the LMBCS-to-Unicode conversion path to prevent stack-buffer overflow when reassembling a multi-byte sequence from a reused converter state.

Changes:

• Validates toULength against the LMBCS reassembly buffer size and returns U_INVALID_STATE_ERROR on invalid state.
• Adds detailed in-code rationale explaining the overflow/arithmetic-wrap risk being mitigated.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[jira-pull-request-webhook]

Notice: the branch changed across the force-push!

• icu4c/source/common/ucnv_lmb.cpp is different

View Diff Across Force-Push

~ Your Friendly Jira-GitHub PR Checker Bot