Skip to content

Fix out-of-bounds reads in Arabic shaping space handling #4043

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
ubeddulla wants to merge 1 commit into
base: main
Choose a base branch
from
+58 −3
Open

Fix out-of-bounds reads in Arabic shaping space handling #4043

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions icu4c/source/common/ushape.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -822,8 +822,8 @@ handleGeneratedSpaces(char16_t *dest, int32_t sourceLength,
if(lamAlefOption || tashkeelOption){
uprv_memset(tempbuffer, 0, (sourceLength+1)*U_SIZEOF_UCHAR);

i = j = sourceLength; count = 0;
i = j = sourceLength-1; count = 0;

while(i >= 0) {
if ( (lamAlefOption && dest[i] == LAMALEF_SPACE_SUB) ||
(tashkeelOption && dest[i] == TASHKEEL_SPACE_SUB) ){
Expand Down Expand Up @@ -1055,7 +1055,7 @@ expandCompositCharAtNear(char16_t *dest, int32_t sourceLength, int32_t destSize,

*pErrorCode=U_NO_SPACE_AVAILABLE;
}
}else if(lamAlefOption && isLamAlefChar(dest[i+1])) {
}else if(lamAlefOption && (i < sourceLength-1) && isLamAlefChar(dest[i+1])) {
if(dest[i] == SPACE_CHAR){
lamalefChar = dest[i+1];
dest[i+1] = LAM_CHAR;
Expand Down
55 changes: 55 additions & 0 deletions icu4c/source/test/cintltst/cbiditst.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,8 @@ static void _testPresentationForms(const UChar *in);

static void doArabicShapingTestForNewCharacters(void);

static void doArabicShapingTestForBufferOverflow(void);

static void testReorder(void);

static void testReorderArabicMathSymbols(void);
Expand Down Expand Up @@ -156,6 +158,7 @@ addComplexTest(TestNode** root) {
addTest(root, testReorderArabicMathSymbols, "complex/bidi/bug-9024");
addTest(root, doArabicShapingTestForBug9024, "complex/arabic-shaping/bug-9024");
addTest(root, doArabicShapingTestForNewCharacters, "complex/arabic-shaping/shaping2");
addTest(root, doArabicShapingTestForBufferOverflow, "complex/arabic-shaping/overflow");
}

static void
Expand Down Expand Up @@ -3174,6 +3177,58 @@ doArabicShapingTestForBug8703(void) {
}
}

static void
doArabicShapingTestForBufferOverflow(void) {
/*
* The internal shaping work buffer holds exactly sourceLength UChars when
* no resizing option is used, so a length above the 300-element stack
* buffer forces a heap allocation of that exact size. The lamalef/tashkeel
* "begin" space handling and the near lamalef expansion used to read one
* element past that buffer; exercise both paths with such a length.
*/
enum { LEN = 400 };
UChar shapeSource[LEN]; /* LAM + ALEF + SPACE + presentation LAM-ALEF */
UChar unshapeSource[LEN]; /* SPACE + presentation LAM-ALEF, ready to expand */
UChar dest[2 * LEN];
UErrorCode errorCode;
int32_t length;
int32_t i;

for(i = 0; i < LEN; ++i) {
switch(i & 3) {
case 0: shapeSource[i] = 0x0644; break; /* LAM */
case 1: shapeSource[i] = 0x0627; break; /* ALEF */
case 2: shapeSource[i] = 0x0020; break; /* SPACE */
default: shapeSource[i] = 0xFEFB; break; /* LAM-ALEF */
}
unshapeSource[i] = (i & 1) ? 0xFEFB : 0x0020;
}

errorCode = U_ZERO_ERROR;
length = u_shapeArabic(shapeSource, LEN, dest, UPRV_LENGTHOF(dest),
U_SHAPE_TEXT_DIRECTION_LOGICAL | U_SHAPE_LAMALEF_BEGIN | U_SHAPE_LETTERS_SHAPE,
&errorCode);
if(U_FAILURE(errorCode) || length > UPRV_LENGTHOF(dest)) {
log_err("failure in u_shapeArabic(LAMALEF_BEGIN, length %d): %s\n", (int)LEN, u_errorName(errorCode));
}

errorCode = U_ZERO_ERROR;
length = u_shapeArabic(shapeSource, LEN, dest, UPRV_LENGTHOF(dest),
U_SHAPE_TEXT_DIRECTION_LOGICAL | U_SHAPE_TASHKEEL_BEGIN | U_SHAPE_LETTERS_SHAPE,
&errorCode);
if(U_FAILURE(errorCode) || length > UPRV_LENGTHOF(dest)) {
log_err("failure in u_shapeArabic(TASHKEEL_BEGIN, length %d): %s\n", (int)LEN, u_errorName(errorCode));
}

errorCode = U_ZERO_ERROR;
length = u_shapeArabic(unshapeSource, LEN, dest, UPRV_LENGTHOF(dest),
U_SHAPE_LAMALEF_NEAR | U_SHAPE_LETTERS_UNSHAPE,
&errorCode);
if(U_FAILURE(errorCode) || length > UPRV_LENGTHOF(dest)) {
log_err("failure in u_shapeArabic(UNSHAPE|LAMALEF_NEAR, length %d): %s\n", (int)LEN, u_errorName(errorCode));
}
}

static void
doArabicShapingTestForBug9024(void) {
static const UChar
Expand Down