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

[wine-bugs at winehq.org]

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

           Summary: In dlls/user32/edit.c EDIT_EM_ReplaceSel Clobbers
                    Important Var When Buffer Overflows
           Product: Wine
           Version: unspecified
          Platform: All
        OS/Version: All
            Status: UNCONFIRMED
          Severity: normal
          Priority: P2
         Component: user32
        AssignedTo: wine-bugs at winehq.org
        ReportedBy: bsmith at sudleyplace.com


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.


-- 
Configure bugmail: http://bugs.winehq.org/userprefs.cgi?tab=email
Do not reply to this email, post in Bugzilla using the
above URL to reply.
------- You are receiving this mail because: -------
You are watching all bug changes.