[Bug 13319] In dlls/user32/edit.c EDIT_EM_ReplaceSel Clobbers Important Var When Buffer Overflows

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

--- Comment #21 from Bob Smith   2008-05-30 20:38:37 ---
Let me try a different tack.  From a static analysis point of view, here's why
I don't understand how the patch can work and how the test case can succeed:

1.  The patch saves the old buffer_limit, tells the parent about the overflow,
but does not again examine the (possibly new) buffer limit to see if
lpsz_replace now fits.  I thought that the purpose of telling the parent about
the overflow is to give her a chance to increase the limit so that the
operation can continue successfully.

2.  In the test case I wrote up, I start with an empty buffer (tl=e=s=0) with a
limit of 30 and replace the (empty) selection with a string of length 40.  In
the patch, <strl> is set to old_limit - (tl - (e-s)) which is 30.

Subsequently, the for loop that copies lpsz_replace to the buffer goes from 0
to strl - 1.  If strl is 30 and lpsz_replace is of length 40, then not all of
lpsz_replace is copied to the buffer.  Back in the test case, I check not only
that the resulting length is 40, but also, if it is, that all 40 of the correct
chars are there.  Obviously we didn't copy all 40 chars to the buffer, so how
can either of those checks succeed?
-----------------------------------------------------
I think both of our fixes have flaws.  One thinks the parent will always
increase the buffer limit to be big enough, and the other thinks she never
will.  I like a combined fix as follows
-----------------------------------------------------
if ((honor_limit) && (size > es->buffer_limit)) {
    EDIT_NOTIFY_PARENT(es, EN_MAXTEXT);
    /* Buffer limit can be smaller than the actual length of text in combobox
*/
    if (es->buffer_limit < (tl - (e-s)))
        strl = 0;
    else
    {
        UINT strl2;

strl2 = es->buffer_limit - (tl - (e-s));
        strl = min (strl, strl2);
    }
}

if (!EDIT_MakeFit(es, tl - (e - s) + strl))
    return;
-----------------------------------------------------
Essentially strl2 is the # available chars in the buffer (after the parent has
had a chance to increase its limit) and strl is the # of chars we are allowed
to insert.