[PATCH v12 0/1] MR10660: uniscribe: Correct possible buffer over/under runs
Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=59532 -- v12: uniscribe: Correct possible buffer over/under runs https://gitlab.winehq.org/wine/wine/-/merge_requests/10660
From: Aric Stewart <aric@codeweavers.com> Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=59532 --- dlls/gdi32/uniscribe/opentype.c | 66 ++++++++++++++++++++++++++------- 1 file changed, 52 insertions(+), 14 deletions(-) diff --git a/dlls/gdi32/uniscribe/opentype.c b/dlls/gdi32/uniscribe/opentype.c index b2a72b8c3ff..1f7643b695d 100644 --- a/dlls/gdi32/uniscribe/opentype.c +++ b/dlls/gdi32/uniscribe/opentype.c @@ -2091,8 +2091,14 @@ 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 +2110,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)) @@ -2206,8 +2212,12 @@ static unsigned int GPOS_apply_ContextPos(const ScriptCache *script_cache, const 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 +2329,12 @@ static unsigned int GPOS_apply_ChainContextPos(const ScriptCache *script_cache, for (k = 0; k < backtrack_count; ++k) { + unsigned int next_glyph_index; offset = GET_BE_WORD(backtrack->Coverage[k]); - if (GSUB_is_glyph_covered((const BYTE *)backtrack + offset, - glyphs[glyph_index + (dirBacktrack * (k + 1))]) == -1) + next_glyph_index = glyph_index + (dirBacktrack * (k + 1)); + if (next_glyph_index >= glyph_count) + continue; + if (GSUB_is_glyph_covered((const BYTE *)backtrack + offset, glyphs[next_glyph_index]) == -1) break; } if (k != backtrack_count) @@ -2330,9 +2343,12 @@ static unsigned int GPOS_apply_ChainContextPos(const ScriptCache *script_cache, for (k = 0; k < input_count; ++k) { + unsigned int next_glyph_index = glyph_index + (write_dir * k); + if (next_glyph_index >= 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[next_glyph_index]) == -1) break; } if (k != input_count) @@ -2341,9 +2357,12 @@ static unsigned int GPOS_apply_ChainContextPos(const ScriptCache *script_cache, for (k = 0; k < lookahead_count; ++k) { + unsigned int next_glyph_index = glyph_index + (dirLookahead * (input_count + k)); + if (next_glyph_index >= 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[next_glyph_index]) == -1) break; } if (k != lookahead_count) @@ -2446,6 +2465,12 @@ 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 next_glyph_index = glyph_index + write_dir; + if (next_glyph_index >= 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 +2488,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[next_glyph_index].du += round(devX) * offset_sign; + pGoffset[next_glyph_index].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[next_glyph_index] += round(devX); } return index_offset; } @@ -2479,13 +2504,19 @@ 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 next_glyph_index = glyph_index + write_dir; + if (next_glyph_index >= 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[next_glyph_index].dv; } break; } @@ -2529,12 +2560,19 @@ 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 next_glyph_index = glyph_index - write_dir; + if (next_glyph_index >= 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[next_glyph_index].du; + pGoffset[glyph_index].dv = round(devY) + pGoffset[next_glyph_index].dv; } break; } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10660
On Wed May 20 11:48:16 2026 +0000, Aric Stewart wrote:
changed this line in [version 12 of the diff](/wine/wine/-/merge_requests/10660/diffs?diff_id=269028&start_sha=fe76965f51eb2f76665dc80d08cd1f7a7ce386dc#a3a84c15578358bf817995d6957408bebf1b7264_2334_2334) Thanks for the catch. Corrected.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/10660#note_140784
On Wed May 20 11:40:58 2026 +0000, Aric Stewart wrote:
And `next` is just meaning next to look at not really next in direction. It all depends on the write_dir if that is to the left or to the right. Since it is - write_dir I can see an argument for `previous_glyph_index` would that be more clear? Perhaps `prev_glyph_index`? It's logically previous no matter which flags are set.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/10660#note_140785
participants (3)
-
Aric Stewart -
Aric Stewart (＠aricstewart)
-
Huw Davies (＠huw)