[PATCH 0/3] MR10859: gdi32/uniscribe: Add bounds checks in GPOS_apply_MarkToMark().

[समीरसिंह Sameer Singh (＠ss141309)]

Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=59680

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/10859

[PATCH 1/3] gdi32/uniscribe: Add bounds checks in GPOS_apply_MarkToMark().

[समीर सिंह Sameer Singh]

From: समीर सिंह Sameer Singh <[e-mail]>

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

[PATCH 2/3] gdi32/uniscribe: Fix off-by-one bounds check in GPOS_apply_MarkToMark().

[समीर सिंह Sameer Singh]

From: समीर सिंह Sameer Singh <[e-mail]>

---
 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

[PATCH 3/3] gdi32/uniscribe: Guard against int overflow in GPOS_apply_MarkToMark().

[समीर सिंह Sameer Singh]

From: समीर सिंह Sameer Singh <[e-mail]>

---
 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

[Stian Low (＠stianlow)]

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

[PATCH 0/3] MR10859: gdi32/uniscribe: Add bounds checks in GPOS_apply_MarkToMark(). - approved

[Aric Stewart (＠aricstewart)]

This merge request was approved by Aric Stewart.
-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/10859