[Bug 37556] New: String compare functions with only one length argument can lead to page faults.

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

Bug ID: 37556
           Summary: String compare functions with only one length argument
                    can lead to page faults.
           Product: Wine
           Version: 1.7.31
          Hardware: x86
                OS: Linux
            Status: UNCONFIRMED
          Severity: normal
          Priority: P2
         Component: -unknown
          Assignee: [e-mail]
          Reporter: [e-mail]
      Distribution: ---

This issue was discovered with Adobe Flash, but is not limited to this single
application. Moreover there are much more ways to trigger this issue than just
the way described below.

Important parts of the stacktrace:

--- snip ---
Unhandled exception: page fault on read access to 0x077ca000 in 32-bit code
(0xf7563773).
[...]
Backtrace:
=>0 0xf7563773 real_length+0x1f(str=".SOL", len=0xf) [libs/wine/sortkey.c:329]
in libwine.so.1 (0x0093cf98)
  1 0xf7563791 wine_compare_string+0x10(flags=0x1, str1=".SOL", len1=0xf,
str2="\?\GLOBALROOT", len2=0xf) [libs/wine/sortkey.c:338] in libwine.so.1
(0x0093cfb8)
  2 0x7b84f65f CompareStringEx+0x1f0(locale=..., flags=..., str1=..., len1=...,
str2=..., len2=..., version=..., reserved=..., lParam=...)
[dlls/kernel32/locale.c:3312] in kernel32 (0x0093d028)
  3 0x7b84f45f CompareStringW+0x3a(lcid=..., flags=..., str1=..., len1=...,
str2=..., len2=...) [dlls/kernel32/locale.c:3271] in kernel32 (0x0093d078)
  4 0x7dd949a3 StrCmpNIW+0x9f(lpszStr=..., lpszComp=..., iLen=...)
[dlls/shlwapi/string.c:418] in shlwapi (0x0093d0d8)
[...]
0xf7563773 real_length+0x1f [libs/wine/sortkey.c:329] in libwine.so.1: movzwl  
 0x0(%eax),%eax
329        while (len && !str[len - 1]) len--;
--- snip ---

What happens here is that StrCmpNIW is called with two strings.

--- snip ---
INT WINAPI StrCmpNIW(LPCWSTR lpszStr, LPCWSTR lpszComp, int iLen)
{
  TRACE("(%s,%s,%i)\n", debugstr_w(lpszStr), debugstr_w(lpszComp), iLen);
  return CompareStringW(GetThreadLocale(), NORM_IGNORECASE, lpszStr, iLen,
lpszComp, iLen) - CSTR_EQUAL;
}
--- snip --

The exact length of both strings is unknown in this function, so wine passes
"iLen" for both strings to CompareStringW. CompareStringW assumes that memory
regions are valid, and then tries to access them in real_length(..). In this
case however unfortunately one of the strings is very close to a page boundary,
so accessing the whole memory block leads to a page fault.

To fix this issue it is either necessary to:

* Change all places where CompareStringW(..) is called with 'invalid' length
values.
* Add exception handlers to real_length(..).
* Move real_length(..) from the beginning of CompareStringW(..) somewhere to
the end, so that the function aborts on the first non-matching byte.

I am opening this issue as a bug report since it needs further investigation,
and I am not sure yet whats the right way to fix it.