Skip to content

Commit 3e7e5d0

Browse files
committed
ICU-23251 Validate serialized spoof data in SpoofData deserialization
Add comprehensive validation for serialized spoof checker data: 1. initPtrs(): Bounds check all offset+size pairs (fCFUKeys, fCFUStringIndex, fCFUStringTable) against the total data length. Validate cross-table consistency: if keys exist, value index and string table must also exist; value index table must have at least as many entries as the keys table. 2. confusableLookup(): Guard against empty confusable data (length == 0 or fCFUKeys == nullptr) to prevent the do-while binary search loop from dereferencing a null pointer. 3. appendValueTo(): Bounds check the string table index before reading from fCFUStrings to prevent heap-buffer-overflow when crafted data contains out-of-range string references. Without these validations, crafted serialized data passed to uspoof_openFromSerialized() causes null pointer dereference, heap buffer overflow reads, and other memory safety issues when the spoof checker is subsequently used.
1 parent a746312 commit 3e7e5d0

2 files changed

Lines changed: 40 additions & 1 deletion

File tree

icu4c/source/common/rbbidata.cpp

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,23 @@ void RBBIDataWrapper::init(const RBBIDataHeader *data, UErrorCode &status) {
102102
// that is no longer supported. At that time fFormatVersion was
103103
// an int32_t field, rather than an array of 4 bytes.
104104

105+
uint32_t totalLen = fHeader->fLength;
106+
if (totalLen < sizeof(RBBIDataHeader)) {
107+
status = U_INVALID_FORMAT_ERROR;
108+
return;
109+
}
110+
111+
// Validate all offset+length pairs against the total data length.
112+
// Prevent integer overflow by checking each addend against totalLen first.
113+
if (fHeader->fFTable > totalLen || fHeader->fFTableLen > totalLen - fHeader->fFTable ||
114+
fHeader->fRTable > totalLen || fHeader->fRTableLen > totalLen - fHeader->fRTable ||
115+
fHeader->fTrie > totalLen || fHeader->fTrieLen > totalLen - fHeader->fTrie ||
116+
fHeader->fRuleSource > totalLen || fHeader->fRuleSourceLen > totalLen - fHeader->fRuleSource ||
117+
fHeader->fStatusTable > totalLen || fHeader->fStatusTableLen > totalLen - fHeader->fStatusTable) {
118+
status = U_INVALID_FORMAT_ERROR;
119+
return;
120+
}
121+
105122
fDontFreeData = false;
106123
if (data->fFTableLen != 0) {
107124
fForwardTable = reinterpret_cast<const RBBIStateTable*>(reinterpret_cast<const char*>(data) + fHeader->fFTable);

icu4c/source/i18n/uspoof_impl.cpp

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -714,9 +714,21 @@ void SpoofData::initPtrs(UErrorCode &status) {
714714
return;
715715
}
716716

717+
// Ensure consistency: offsets and sizes must agree.
717718
// If keys exist, the value index and string table must also exist.
718719
if (fRawData->fCFUKeysSize > 0 &&
719-
(fRawData->fCFUStringIndex == 0 || fRawData->fCFUStringTable == 0)) {
720+
(fRawData->fCFUKeys == 0 || fRawData->fCFUStringIndex == 0 || fRawData->fCFUStringTable == 0)) {
721+
status = U_INVALID_FORMAT_ERROR;
722+
return;
723+
}
724+
// If offset is zero, size must also be zero.
725+
if ((fRawData->fCFUKeys == 0) != (fRawData->fCFUKeysSize == 0)) {
726+
status = U_INVALID_FORMAT_ERROR;
727+
return;
728+
}
729+
// Value index table must have at least as many entries as the keys table,
730+
// since both are accessed by the same index from binary search.
731+
if (fRawData->fCFUKeysSize > 0 && fRawData->fCFUStringIndexSize < fRawData->fCFUKeysSize) {
720732
status = U_INVALID_FORMAT_ERROR;
721733
return;
722734
}
@@ -802,6 +814,10 @@ int32_t SpoofData::confusableLookup(UChar32 inChar, UnicodeString &dest) const {
802814
// The result after the loop will be in lo.
803815
int32_t lo = 0;
804816
int32_t hi = length();
817+
if (hi == 0 || fCFUKeys == nullptr) {
818+
dest.append(inChar);
819+
return 1;
820+
}
805821
do {
806822
int32_t mid = (lo + hi) / 2;
807823
if (codePointAt(mid) > inChar) {
@@ -842,6 +858,12 @@ int32_t SpoofData::appendValueTo(int32_t index, UnicodeString& dest) const {
842858
if (stringLength == 1) {
843859
dest.append(static_cast<char16_t>(value));
844860
} else {
861+
int32_t tableLen = fRawData->fCFUStringTableLen;
862+
if (fCFUStrings == nullptr || value > tableLen ||
863+
stringLength > tableLen - value) {
864+
dest.append(static_cast<UChar32>(0xFFFD));
865+
return 1;
866+
}
845867
dest.append(fCFUStrings + value, stringLength);
846868
}
847869

0 commit comments

Comments
 (0)