From 476565357768439905b9f75347e31cd16fe6ea1c Mon Sep 17 00:00:00 2001 From: Peter Edberg Date: Wed, 1 Feb 2017 20:27:47 +0000 Subject: [PATCH] ICU-12914 change rulesLength/Capacity back to int32_t; handle INT32_MAX overflow X-SVN-Rev: 39630 --- icu4c/source/common/ubrk.cpp | 21 +++++++++++++++------ icu4c/source/common/unicode/ubrk.h | 25 +++++++++++++++---------- icu4c/source/test/cintltst/cbiapts.c | 2 +- 3 files changed, 31 insertions(+), 17 deletions(-) diff --git a/icu4c/source/common/ubrk.cpp b/icu4c/source/common/ubrk.cpp index 06efae9c86c..f8bdf5a6b65 100644 --- a/icu4c/source/common/ubrk.cpp +++ b/icu4c/source/common/ubrk.cpp @@ -121,13 +121,17 @@ ubrk_openRules( const UChar *rules, U_CAPI UBreakIterator* U_EXPORT2 -ubrk_openBinaryRules(const uint8_t *binaryRules, uint32_t rulesLength, +ubrk_openBinaryRules(const uint8_t *binaryRules, int32_t rulesLength, const UChar * text, int32_t textLength, UErrorCode * status) { if (U_FAILURE(*status)) { return NULL; } + if (rulesLength < 0) { + *status = U_ILLEGAL_ARGUMENT_ERROR; + return NULL; + } LocalPointer lpRBBI(new RuleBasedBreakIterator(binaryRules, rulesLength, *status), *status); if (U_FAILURE(*status)) { return NULL; @@ -315,15 +319,15 @@ ubrk_refreshUText(UBreakIterator *bi, bii->refreshInputText(text, *status); } -U_CAPI uint32_t U_EXPORT2 +U_CAPI int32_t U_EXPORT2 ubrk_getBinaryRules(UBreakIterator *bi, - uint8_t * binaryRules, uint32_t rulesCapacity, + uint8_t * binaryRules, int32_t rulesCapacity, UErrorCode * status) { if (U_FAILURE(*status)) { return 0; } - if (binaryRules == NULL && rulesCapacity > 0) { + if ((binaryRules == NULL && rulesCapacity > 0) || rulesCapacity < 0) { *status = U_ILLEGAL_ARGUMENT_ERROR; return 0; } @@ -334,14 +338,19 @@ ubrk_getBinaryRules(UBreakIterator *bi, } uint32_t rulesLength; const uint8_t * returnedRules = rbbi->getBinaryRules(rulesLength); + if (rulesLength > INT32_MAX) { + *status = U_INDEX_OUTOFBOUNDS_ERROR; + return 0; + } if (binaryRules != NULL) { // if not preflighting - if (rulesLength > rulesCapacity) { + // Here we know rulesLength <= INT32_MAX and rulesCapacity >= 0, can cast safely + if ((int32_t)rulesLength > rulesCapacity) { *status = U_BUFFER_OVERFLOW_ERROR; } else { uprv_memcpy(binaryRules, returnedRules, rulesLength); } } - return rulesLength; + return (int32_t)rulesLength; } diff --git a/icu4c/source/common/unicode/ubrk.h b/icu4c/source/common/unicode/ubrk.h index 5210bb0b5b9..22a4b99cd6d 100644 --- a/icu4c/source/common/unicode/ubrk.h +++ b/icu4c/source/common/unicode/ubrk.h @@ -279,7 +279,7 @@ ubrk_openRules(const UChar *rules, * rules remains with the caller of this function. The compiled * rules must not be modified or deleted during the life of the * break iterator. - * @param rulesLength The length of binaryRules in bytes. + * @param rulesLength The length of binaryRules in bytes; must be >= 0. * @param text The text to be iterated over. May be null, in which case * ubrk_setText() is used to specify the text to be iterated. * @param textLength The number of characters in text, or -1 if null-terminated. @@ -289,7 +289,7 @@ ubrk_openRules(const UChar *rules, * @draft ICU 59 */ U_DRAFT UBreakIterator* U_EXPORT2 -ubrk_openBinaryRules(const uint8_t *binaryRules, uint32_t rulesLength, +ubrk_openBinaryRules(const uint8_t *binaryRules, int32_t rulesLength, const UChar * text, int32_t textLength, UErrorCode * status); @@ -603,22 +603,27 @@ ubrk_refreshUText(UBreakIterator *bi, * different major versions of ICU, nor across platforms of different endianness or * different base character set family (ASCII vs EBCDIC). Supports preflighting (with * binaryRules=NULL and rulesCapacity=0) to get the rules length without copying them to - * the binaryRules buffer, + * the binaryRules buffer. However, whether preflighting or not, if the actual length + * is greater than INT32_MAX, then the function returns 0 and sets *status to + * U_INDEX_OUTOFBOUNDS_ERROR. + * @param bi The break iterator to use. * @param binaryRules Buffer to receive the compiled binary rules; set to NULL for * preflighting. * @param rulesCapacity Capacity (in bytes) of the binaryRules buffer; set to 0 for - * preflighting. - * @param status Pointer to UErrorCode to receive any errors. - * @return The actual byte length of the binary rules. If not preflighting - * and this is larger than rulesCapacity, *status will be set to - * an error. + * preflighting. Must be >= 0. + * @param status Pointer to UErrorCode to receive any errors, such as + * U_BUFFER_OVERFLOW_ERROR, U_INDEX_OUTOFBOUNDS_ERROR, or + * U_ILLEGAL_ARGUMENT_ERROR. + * @return The actual byte length of the binary rules, if <= INT32_MAX; + * otherwise 0. If not preflighting and this is larger than + * rulesCapacity, *status will be set to an error. * @see ubrk_openBinaryRules * @draft ICU 59 */ -U_DRAFT uint32_t U_EXPORT2 +U_DRAFT int32_t U_EXPORT2 ubrk_getBinaryRules(UBreakIterator *bi, - uint8_t * binaryRules, uint32_t rulesCapacity, + uint8_t * binaryRules, int32_t rulesCapacity, UErrorCode * status); #endif /* U_HIDE_DRAFT_API */ diff --git a/icu4c/source/test/cintltst/cbiapts.c b/icu4c/source/test/cintltst/cbiapts.c index 1b58e1051c3..ec4fc838c07 100644 --- a/icu4c/source/test/cintltst/cbiapts.c +++ b/icu4c/source/test/cintltst/cbiapts.c @@ -589,7 +589,7 @@ static void TestBreakIteratorRules() { /* #12914 add basic sanity test for ubrk_getBinaryRules, ubrk_openBinaryRules */ /* Underlying functionality checked in C++ rbbiapts.cpp TestRoundtripRules */ status = U_ZERO_ERROR; - uint32_t rulesLength = ubrk_getBinaryRules(bi, NULL, 0, &status); /* preflight */ + int32_t rulesLength = ubrk_getBinaryRules(bi, NULL, 0, &status); /* preflight */ if (U_FAILURE(status)) { log_err("FAIL: ubrk_getBinaryRules preflight err: %s", u_errorName(status)); } else {