[PATCH 0/3] MR10859: gdi32/uniscribe: Add bounds checks in GPOS_apply_MarkToMark().
From: समीर सिंह Sameer Singh <lumarzeli30@gmail.com> Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=59680 --- dlls/gdi32/uniscribe/opentype.c | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/dlls/gdi32/uniscribe/opentype.c b/dlls/gdi32/uniscribe/opentype.c index b2a72b8c3ff..b4eadb6ca44 100644 --- a/dlls/gdi32/uniscribe/opentype.c +++ b/dlls/gdi32/uniscribe/opentype.c @@ -2080,6 +2080,13 @@ static BOOL GPOS_apply_MarkToMark(const OT_LookupTable *look, const SCRIPT_ANALY TRACE("MarkToMark Attachment Positioning Subtable\n"); + if ((write_dir == 1 && glyph_index == 0) || + (write_dir == -1 && glyph_index + 1 == glyph_count)) + { + ERR("Out of bounds access in glyphs array\n"); + return FALSE; + } + for (j = 0; j < GET_BE_WORD(look->SubTableCount); j++) { const GPOS_MarkMarkPosFormat1 *mmpf1 = (const GPOS_MarkMarkPosFormat1 *)GPOS_get_subtable(look, j); @@ -2115,8 +2122,18 @@ static BOOL GPOS_apply_MarkToMark(const OT_LookupTable *look, const SCRIPT_ANALY mr = &ma->MarkRecord[mark_index]; mark_class = GET_BE_WORD(mr->Class); TRACE("Mark Class %i total classes %i\n",mark_class,class_count); + if (mark_class >= class_count) + { + ERR("Mark class exceeded total classes\n"); + return FALSE; + } offset = GET_BE_WORD(mmpf1->Mark2Array); m2a = (const GPOS_Mark2Array*)((const BYTE*)mmpf1 + offset); + if (mark2_index >= GET_BE_WORD(m2a->Mark2Count)) + { + ERR("Mark2 index exceeded mark2 count\n"); + return FALSE; + } mark2record_size = class_count * sizeof(WORD); m2r = (const GPOS_Mark2Record*)((const BYTE*)m2a + sizeof(WORD) + (mark2record_size * mark2_index)); offset = GET_BE_WORD(m2r->Mark2Anchor[mark_class]); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10859
From: समीर सिंह Sameer Singh <lumarzeli30@gmail.com> --- dlls/gdi32/uniscribe/opentype.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dlls/gdi32/uniscribe/opentype.c b/dlls/gdi32/uniscribe/opentype.c index b4eadb6ca44..07fdc53d284 100644 --- a/dlls/gdi32/uniscribe/opentype.c +++ b/dlls/gdi32/uniscribe/opentype.c @@ -2114,7 +2114,7 @@ static BOOL GPOS_apply_MarkToMark(const OT_LookupTable *look, const SCRIPT_ANALY TRACE("Mark %x(%i) and Mark2 %x(%i)\n",glyphs[glyph_index], mark_index, glyphs[glyph_index - write_dir], mark2_index); offset = GET_BE_WORD(mmpf1->Mark1Array); ma = (const GPOS_MarkArray*)((const BYTE*)mmpf1 + offset); - if (mark_index > GET_BE_WORD(ma->MarkCount)) + if (mark_index >= GET_BE_WORD(ma->MarkCount)) { ERR("Mark index exceeded mark count\n"); return FALSE; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10859
From: समीर सिंह Sameer Singh <lumarzeli30@gmail.com> --- dlls/gdi32/uniscribe/opentype.c | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/dlls/gdi32/uniscribe/opentype.c b/dlls/gdi32/uniscribe/opentype.c index 07fdc53d284..ae4ab322c65 100644 --- a/dlls/gdi32/uniscribe/opentype.c +++ b/dlls/gdi32/uniscribe/opentype.c @@ -2109,6 +2109,7 @@ static BOOL GPOS_apply_MarkToMark(const OT_LookupTable *look, const SCRIPT_ANALY int mark_class; int class_count = GET_BE_WORD(mmpf1->ClassCount); int mark2record_size; + int mark2_offset; POINT mark2_pt; POINT mark_pt; TRACE("Mark %x(%i) and Mark2 %x(%i)\n",glyphs[glyph_index], mark_index, glyphs[glyph_index - write_dir], mark2_index); @@ -2135,7 +2136,18 @@ static BOOL GPOS_apply_MarkToMark(const OT_LookupTable *look, const SCRIPT_ANALY return FALSE; } mark2record_size = class_count * sizeof(WORD); - m2r = (const GPOS_Mark2Record*)((const BYTE*)m2a + sizeof(WORD) + (mark2record_size * mark2_index)); + if (mark2_index > 0 && mark2record_size > INT_MAX / mark2_index) + { + ERR("Integer overflow in mark2 record size\n"); + return FALSE; + } + mark2_offset = mark2record_size * mark2_index; + if (mark2_offset > INT_MAX - sizeof(WORD)) + { + ERR("Integer overflow in mark2 record size\n"); + return FALSE; + } + m2r = (const GPOS_Mark2Record*)((const BYTE*)m2a + sizeof(WORD) + mark2_offset); offset = GET_BE_WORD(m2r->Mark2Anchor[mark_class]); GPOS_get_anchor_values((const BYTE*)m2a + offset, &mark2_pt, ppem); offset = GET_BE_WORD(mr->MarkAnchor); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10859
Success reported for GE-Proton10-32 after replacing gdi32.dll: https://bugs.winehq.org/show_bug.cgi?id=59680#c17 Still unclear why I am unable to reproduce the crash reported for my system wine builds but GE-Proton10-32 no longer crashing seems proof of a fix. Besides these errors that are logged, no other obvious misbehavior seems to happen: ``` 00d8:err:uniscribe:GPOS_apply_MarkToMark Out of bounds access in glyphs array 00d8:err:uniscribe:GPOS_apply_MarkToMark Out of bounds access in glyphs array 00d8:err:uniscribe:GPOS_apply_MarkToMark Out of bounds access in glyphs array ``` -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10859#note_139385
participants (3)
-
Stian Low (＠stianlow) -
समीर सिंह Sameer Singh
-
समीरसिंह Sameer Singh (＠ss141309)