[PATCH v2 0/1] MR10660: uniscribe: Correct possible buffer over/under runs

[Aric Stewart (＠aricstewart)]

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

--
  v2: uniscribe: Correct possible buffer over/under runs
 
https://gitlab.winehq.org/wine/wine/-/merge_requests/10660

[PATCH v2 1/1] uniscribe: Correct possible buffer over/under runs

[Aric Stewart]

From: Aric Stewart <[e-mail]>

Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=59532
---
 dlls/gdi32/uniscribe/opentype.c | 63 +++++++++++++++++++++++++--------
 1 file changed, 48 insertions(+), 15 deletions(-)

diff --git a/dlls/gdi32/uniscribe/opentype.c b/dlls/gdi32/uniscribe/opentype.c
index b2a72b8c3ff..fea63fa01c4 100644
--- a/dlls/gdi32/uniscribe/opentype.c
+++ b/dlls/gdi32/uniscribe/opentype.c
@@ -2091,8 +2091,13 @@ static BOOL GPOS_apply_MarkToMark(const OT_LookupTable *look, const SCRIPT_ANALY
             if (mark_index != -1)
             {
                 int mark2_index;
+                unsigned int next_glyph_index = glyph_index - write_dir;
+                if (next_glyph_index >= glyph_count) {
+                    TRACE("Mark looking outside range\n");
+                    return FALSE;
+                }
                 offset = GET_BE_WORD(mmpf1->Mark2Coverage);
-                mark2_index = GSUB_is_glyph_covered((const BYTE*)mmpf1+offset, glyphs[glyph_index - write_dir]);
+                mark2_index = GSUB_is_glyph_covered((const BYTE*)mmpf1+offset, glyphs[next_glyph_index]);
                 if (mark2_index != -1)
                 {
                     const GPOS_MarkArray *ma;
@@ -2104,7 +2109,7 @@ static BOOL GPOS_apply_MarkToMark(const OT_LookupTable *look, const SCRIPT_ANALY
                     int mark2record_size;
                     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);
+                    TRACE("Mark %x(%i) and Mark2 %x(%i)\n",glyphs[glyph_index], mark_index, glyphs[next_glyph_index], mark2_index);
                     offset = GET_BE_WORD(mmpf1->Mark1Array);
                     ma = (const GPOS_MarkArray*)((const BYTE*)mmpf1 + offset);
                     if (mark_index > GET_BE_WORD(ma->MarkCount))
@@ -2201,13 +2206,16 @@ static unsigned int GPOS_apply_ContextPos(const ScriptCache *script_cache, const
                     TRACE("PosClassRule has %i glyphs classes\n",g_count);
 
                     g = glyph_index + write_dir * (g_count - 1);
-                    if (g >= glyph_count)
+                    if (g < 0 || g >= glyph_count)
                         continue;
 
                     for (l = 0; l < g_count-1; l++)
                     {
-                        int g_class = OT_get_glyph_class(glyph_class_table, glyphs[glyph_index + (write_dir * (l+1))]);
-                        if (g_class != GET_BE_WORD(pr->Class[l])) break;
+                        g = glyph_index + (write_dir * (l+1));
+                        if (g < glyph_count) {
+                            int g_class = OT_get_glyph_class(glyph_class_table, glyphs[g]);
+                            if (g_class != GET_BE_WORD(pr->Class[l])) break;
+                        }
                     }
 
                     if (l < g_count-1)
@@ -2319,9 +2327,12 @@ static unsigned int GPOS_apply_ChainContextPos(const ScriptCache *script_cache,
 
             for (k = 0; k < backtrack_count; ++k)
             {
+                unsigned int j;
                 offset = GET_BE_WORD(backtrack->Coverage[k]);
-                if (GSUB_is_glyph_covered((const BYTE *)backtrack + offset,
-                        glyphs[glyph_index + (dirBacktrack * (k + 1))]) == -1)
+                j = glyph_index + (dirBacktrack * (k + 1));
+                if (j >= glyph_count)
+                    continue;
+                if (GSUB_is_glyph_covered((const BYTE *)backtrack + offset, glyphs[j]) == -1)
                     break;
             }
             if (k != backtrack_count)
@@ -2330,9 +2341,12 @@ static unsigned int GPOS_apply_ChainContextPos(const ScriptCache *script_cache,
 
             for (k = 0; k < input_count; ++k)
             {
+                unsigned int j = glyph_index + (write_dir * k);
+                if (j >= glyph_count)
+                    continue;
                 offset = GET_BE_WORD(input->Coverage[k]);
                 if (GSUB_is_glyph_covered((const BYTE *)backtrack + offset,
-                        glyphs[glyph_index + (write_dir * k)]) == -1)
+                        glyphs[j]) == -1)
                     break;
             }
             if (k != input_count)
@@ -2341,9 +2355,12 @@ static unsigned int GPOS_apply_ChainContextPos(const ScriptCache *script_cache,
 
             for (k = 0; k < lookahead_count; ++k)
             {
+                unsigned int j = glyph_index + (dirLookahead * (input_count + k));
+                if (j >= glyph_count)
+                    continue;
                 offset = GET_BE_WORD(lookahead->Coverage[k]);
                 if (GSUB_is_glyph_covered((const BYTE *)backtrack + offset,
-                        glyphs[glyph_index + (dirLookahead * (input_count + k))]) == -1)
+                        glyphs[j]) == -1)
                     break;
             }
             if (k != lookahead_count)
@@ -2446,6 +2463,11 @@ static unsigned int GPOS_apply_lookup(const ScriptCache *script_cache, const OUT
             int index_offset;
             int write_dir = (analysis->fRTL && !analysis->fLogicalOrder) ? -1 : 1;
             int offset_sign = (analysis->fRTL && analysis->fLogicalOrder) ? -1 : 1;
+            unsigned int k = glyph_index + write_dir;
+            if (k >= glyph_count) {
+                WARN("Skipping invalid glyph index %u, write dir %d.\n", glyph_index, write_dir);
+                break;
+            }
 
             index_offset = GPOS_apply_PairAdjustment(look, analysis, glyphs,
                     glyph_index, glyph_count, ppem, adjust, advance);
@@ -2463,13 +2485,13 @@ static unsigned int GPOS_apply_lookup(const ScriptCache *script_cache, const OUT
             if (adjust[1].x || adjust[1].y)
             {
                 GPOS_convert_design_units_to_device(lpotm, lplogfont, adjust[1].x, adjust[1].y, &devX, &devY);
-                pGoffset[glyph_index + write_dir].du += round(devX) * offset_sign;
-                pGoffset[glyph_index + write_dir].dv += round(devY);
+                pGoffset[k].du += round(devX) * offset_sign;
+                pGoffset[k].dv += round(devY);
             }
             if (advance[1].x || advance[1].y)
             {
                 GPOS_convert_design_units_to_device(lpotm, lplogfont, advance[1].x, advance[1].y, &devX, &devY);
-                piAdvance[glyph_index + write_dir] += round(devX);
+                piAdvance[k] += round(devX);
             }
             return index_offset;
         }
@@ -2479,13 +2501,18 @@ static unsigned int GPOS_apply_lookup(const ScriptCache *script_cache, const OUT
             POINT desU = {0,0};
             double devX, devY;
             int write_dir = (analysis->fRTL && !analysis->fLogicalOrder) ? -1 : 1;
+            unsigned int k = glyph_index + write_dir;
+            if (k >= glyph_count) {
+                WARN("Skipping invalid glyph index %u, write dir %d.\n", glyph_index, write_dir);
+                break;
+            }
 
             GPOS_apply_CursiveAttachment(look, analysis, glyphs, glyph_index, glyph_count, ppem, &desU);
             if (desU.x || desU.y)
             {
                 GPOS_convert_design_units_to_device(lpotm, lplogfont, desU.x, desU.y, &devX, &devY);
                 /* Windows does not appear to apply X offsets here */
-                pGoffset[glyph_index].dv = round(devY) + pGoffset[glyph_index+write_dir].dv;
+                pGoffset[glyph_index].dv = round(devY) + pGoffset[k].dv;
             }
             break;
         }
@@ -2529,12 +2556,18 @@ static unsigned int GPOS_apply_lookup(const ScriptCache *script_cache, const OUT
             double devX, devY;
             POINT desU = {0,0};
             int write_dir = (analysis->fRTL && !analysis->fLogicalOrder) ? -1 : 1;
+            unsigned int k = glyph_index - write_dir;
+            if (k >= glyph_count) {
+                WARN("Skipping invalid glyph index %u, write dir %d.\n", glyph_index, write_dir);
+                break;
+            }
+
             if (GPOS_apply_MarkToMark(look, analysis, glyphs, glyph_index, glyph_count, ppem, &desU))
             {
                 GPOS_convert_design_units_to_device(lpotm, lplogfont, desU.x, desU.y, &devX, &devY);
                 if (analysis->fRTL && analysis->fLogicalOrder) devX *= -1;
-                pGoffset[glyph_index].du = round(devX) + pGoffset[glyph_index - write_dir].du;
-                pGoffset[glyph_index].dv = round(devY) + pGoffset[glyph_index - write_dir].dv;
+                pGoffset[glyph_index].du = round(devX) + pGoffset[k].du;
+                pGoffset[glyph_index].dv = round(devY) + pGoffset[k].dv;
             }
             break;
         }
-- 
GitLab

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