Bug in dlls/user32/edit.c

[Bob Smith]

Please don't hate me, but I don't have git or anything like it installed 
to make a formal patch, but I have benefited greatly from your work, so 
I'd like to repay the effort by reporting a bug even though I realize 
it's not in the correct format.

In a Windows app, I am using edit.c as a replacement for the EDIT 
control in Windows.  I don't use any of the other WineHQ files.

Here's a short description of the bug:

The handler EDIT_EM_ReplaceSel misbehaves when an insertion triggers a 
buffer overflow.  The code correctly calls EDIT_NOTIFY_PARENT(es, 
EN_MAXTEXT), but shortly thereafter clobbers an important variable 
(strl) by using it instead of a temporary.

The relevant old code in EDIT_EM_ReplaceSel is 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
             strl = es->buffer_limit - (tl - (e-s));
     }

     if (!EDIT_MakeFit(es, tl - (e - s) + strl))
         return;
--------------------------------------------------------------
the new code is 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)))
             strl2 = 0;
         else
             strl2 = es->buffer_limit - (tl - (e-s));
     }
     else
         strl2 = strl;

     if (!EDIT_MakeFit(es, tl - (e - s) + strl2))
         return;
--------------------------------------------------------------
The calculation inside the honor_limit bracket of the value to use with 
the call to EDIT_MakeFit uses strl as if it were a temp var.  This 
variable actually holds strlenW (lpsz_replace) and is used in later code 
as if it still had the original value.  Using a (new) variable strl2 
solves that problem -- this variable is declared as a UINT in the 
prologue.  Perhaps you would prefer a name different from strl2 to 
better reflect its temporary nature.

If you agree with the above analysis, I would greatly appreciate it if 
someone would make this into a patch and take it from there.  I have 
make the above changes in my copy of edit.c and it works just fine when 
the buffer overflows which is how I stumbled on this bug in the first place.

Many thanks for all your great work.  It has saved me much grief as the 
Windows EDIT control has too few hooks and even fewer smarts.

-- 
_______________________________________________________________
Bob Smith - bsmith at sudleyplace.com - http://www.sudleyplace.com