[Bug 32495] New: Incorrect behavior in ScriptGetLogicalWidths() / ScriptApplyLogicalWidth() (buffer overrun)

http://bugs.winehq.org/show_bug.cgi?id=32495

Bug #: 32495
           Summary: Incorrect behavior in ScriptGetLogicalWidths() /
                    ScriptApplyLogicalWidth() (buffer overrun)
           Product: Wine
           Version: 1.5.19
          Platform: x86
        OS/Version: Linux
            Status: UNCONFIRMED
          Severity: normal
          Priority: P2
         Component: usp10
        AssignedTo: [e-mail]
        ReportedBy: [e-mail]
    Classification: Unclassified

Currently, ScriptGetLogicalWidths() and ScriptApplyLogicalWidth() are
incorrectly implemented, leading to wrong glyph positioning in one of my
applications.

**ScriptGetLogicalWidths(): The current implementation simply copies the input
to the output array, incorrectly assuming nbchar==nbglyphs. If there are fewer
glyphs than characters, this causes a buffer overrun, potentially leading to
application crashes. (The problem does and will become more prevalent as the
implementation of shaping is improved over time.)

**ScriptApplyLogicalWidths(): The functions fails to apply justification and
simply returns the unjustified widths.

Below I include what I believe are correct implementations. I have taken care
that the results agree with those returned by the Microsoft implementations. (I
have never seen or been in contact with the original Microsoft code, though, so
the following code is "clean" and may be used for any purpose.)

HRESULT WINAPI ScriptGetLogicalWidths(const SCRIPT_ANALYSIS *sa, int nbchars,
int nbglyphs,
                                      const int *glyph_width, const WORD
*log_clust,
                                      const SCRIPT_VISATTR *sva, int *widths)
{
    int rtl, i;

//    TRACE("(%p, %d, %d, %p, %p, %p, %p)\n",
//          sa, nbchars, nbglyphs, glyph_width, log_clust, sva, widths);

rtl = sa->fRTL && !sa->fLogicalOrder;

for (i = 0; i < nbchars; )
    {
        int w = 0, i2, j, j2;

j = log_clust[i];
        i2 = i;
        do i2++; while (i2 < nbchars && log_clust[i2] == j);
        j2 = i2 < nbchars ? log_clust[i2] : rtl? -1 : nbglyphs;

for ( ; j != j2; rtl? j--:j++)
            w += glyph_width[j];

for ( ; i < i2; i++)
            w -= widths[i] = w / (i2-i);
    }

return S_OK;
}

HRESULT WINAPI ScriptApplyLogicalWidth(const int *dx, int num_chars, int
num_glyphs,
                                       const WORD *log_clust, const
SCRIPT_VISATTR *sva,
                                       const int *advance, const
SCRIPT_ANALYSIS *sa,
                                       ABC *abc, int *justify)
{

int rtl, i;

//    FIXME("(%p, %d, %d, %p, %p, %p, %p, %p, %p)\n",
//          dx, num_chars, num_glyphs, log_clust, sva, advance, sa, abc,
justify);

rtl = sa->fRTL && !sa->fLogicalOrder;

if (abc) abc->abcB = - abc->abcA - abc->abcC;

for (i = 0; i < num_chars; )
    {
        int w = 0, j, j1, j2;

j1 = log_clust[i];
        do w += dx[i++]; while (i < num_chars && log_clust[i] == j1);
        j2 = i < num_chars ? log_clust[i] : rtl? -1 : num_glyphs;

for (j = j1; j != j2; rtl? j--:j++)
            w -= advance[j];

for (j = j1; j != j2; rtl? j--:j++)
        {
            w -= justify[j] = w / (rtl? j-j2:j2-j);
            justify[j] += advance[j];
            if (abc) abc->abcB += justify[j];
        }
    }

return S_OK;
}

[Bug 32495] Incorrect behavior in ScriptGetLogicalWidths() / ScriptApplyLogicalWidth() (buffer overrun)

https://bugs.winehq.org/show_bug.cgi?id=32495

Qian Hong  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |download, source
                 CC|                            |[e-mail]

--- Comment #1 from Qian Hong  ---
Does it still present in lastest Wine (1.7.32 or later) ?

If true could you read http://wiki.winehq.org/SubmittingPatches , add test case
and submit patches to Wine by yourself?

Thanks!

[Bug 32495] Incorrect behavior in ScriptGetLogicalWidths() / ScriptApplyLogicalWidth() (buffer overrun)

https://bugs.winehq.org/show_bug.cgi?id=32495

[e-mail] changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |[e-mail]

--- Comment #2 from [e-mail] ---
Not taking part of the correctness of the patch, but it's still valid
1.7.51(git)

[Bug 32495] Incorrect behavior in ScriptGetLogicalWidths() / ScriptApplyLogicalWidth() (buffer overrun)

https://bugs.winehq.org/show_bug.cgi?id=32495

[e-mail] changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |[e-mail]
                   |                            |m

[Bug 32495] Incorrect behavior in ScriptGetLogicalWidths() / ScriptApplyLogicalWidth() (buffer overrun)

https://bugs.winehq.org/show_bug.cgi?id=32495

--- Comment #3 from Nikolay Sivov  ---
First part is fixed with
http://source.winehq.org/git/wine.git/?a=commit;h=0695368406b54c97e6f6d3b326....