[Bug 13319] 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





--- Comment #4 from Luke Bratch <l_bratch at yahoo.co.uk>  2008-05-28 20:06:29 ---
Bob, please do not reply to wine-bugs, post in Bugzilla instead.  Email read:

"Attached is a C file which should demonstrate the problem.  You'll need to
adapt it to your environment as I use EDIT.C standalone.  In particular, you'll
need to change the value of LECWNDCLASS to the correct class name.

Perhaps the best way to see what's happening is to insert lines into the EDIT.C
function EDIT_EM_ReplaceSel:

Just before the lines

for (i = 0 , p = es->text + s ; i < strl ; i++)
     p[i] = lpsz_replace[i];

insert the lines

if (strl > strlenW (lpsz_replace))
    MessageBoxW (es->hwndParent,
                 L"Indexing out of bounds",
                 L"EDIT_EM_ReplaceSel",
                 MB_OK | MB_ICONEXCLAMATION);

Obviously, when strl retains its original value of strlenW(lpsz_replace), the
FOR loop indexes lpsz_replace from start to finish.  Thus, if the MessageBoxW
statement triggers, we're about to index lpsz_replace beyond its bounds.

-----------------------------------------------------

On related point, when the buffer overflows, the parent gets one shot to set
new limits.  Moreover, it might have no way of knowing how close it is to the
buffer limit, so it's hard to decide by how much the limit should be increased.
 It occurred to me that a small change in the code could resolve this problem:

Just after the line

if ((honor_limit) && (size > es->buffer_limit)) {

replace the call to EDIT_NOTIFY_PARENT(es, EN_MAXTEXT); with

    while (size > es->buffer_limit) {
        UINT oldLimit = es->buffer_limit;
        EDIT_NOTIFY_PARENT(es, EN_MAXTEXT);
        // If the new limit didn't increase, don't try again
        if (oldLimit >= es->buffer_limit)
            break;
    }

This way, the caller gets notified until the buffer limit is big enough or the
buffer limit doesn't change.

-----------------------------------------------------

FWIW, in EDIT.C I also found three typos/bugs.

#1:

old

ExStyle = GetWindowLongPtrW(es->hwndSelf, GWL_EXSTYLE);

new

ExStyle = GetWindowLongW(es->hwndSelf, GWL_EXSTYLE);

That is, GWL_EXSTYLE is a long, not a long ptr.

#2:

old

hBrush = (HBRUSH) GetClassLongW (es->hwndSelf, GCL_HBRBACKGROUND);

new

hBrush = (HBRUSH) GetClassLongPtrW (es->hwndSelf, GCL_HBRBACKGROUND);

That is, GCL_HBACKGROUND is a ptr, not a long.

#3:

old

MAKEWPARAM(GetWindowLongPtrW((es->hwndSelf),GWLP_ID), wNotifyCode), \

new

MAKEWPARAM(GetWindowLongW((es->hwndSelf),GWLP_ID), wNotifyCode), \

I thought that the GWL_ID was an integer (although it often is cast with
(HMENU)).  Nonetheless, MAKEWPARAM is expecting two four-byte values.
Under _WIN64, the difference is important."


-- 
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.