[Bug 22020] New: Page Fault in wine_utf8_wcstombs when running ToonTalk

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

Summary: Page Fault in wine_utf8_wcstombs when running ToonTalk
           Product: Wine
           Version: 1.1.40
          Platform: x86
               URL: http://www.toontalk.com/English/free.htm
        OS/Version: Linux
            Status: UNCONFIRMED
          Severity: normal
          Priority: P2
         Component: -unknown
        AssignedTo: [e-mail]
        ReportedBy: [e-mail]

Created an attachment (id=26770)
 --> (http://bugs.winehq.org/attachment.cgi?id=26770)
Debug trace

When running a game called ToonTalk (full free download @ URL above) it crashes
with a page fault on read, I've attached full debug trace.

I'm happy to run more debugging if someone lets me know what options, etc.

[Bug 22020] Page Fault in wine_utf8_wcstombs when running ToonTalk

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

Austin English  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |download

--- Comment #1 from Austin English   2010-03-12 14:43:37 ---
Please install debugging symbols and attach another backtrace.

[Bug 22020] Page Fault in wine_utf8_wcstombs when running ToonTalk

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

Dmitry Timoshkov  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
          Component|-unknown                    |msxml3

--- Comment #2 from Dmitry Timoshkov   2010-03-13 06:06:38 ---
Does installing msxml3 with winetrics help?

[Bug 22020] Page Fault in wine_utf8_wcstombs when running ToonTalk

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

--- Comment #3 from Peter Belm   2010-03-13 06:37:47 ---
Installing msxml3 solves the problem. I'm going to attach a full debug trace so
the problem with Wine can be fixed. wine_utf8_wcstombs is passing an empty
string to get_length_wcs_utf causing a buffer overflow as far as I can tell.

[Bug 22020] Page Fault in wine_utf8_wcstombs when running ToonTalk

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

Peter Belm  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #26770|0                           |1
        is obsolete|                            |

--- Comment #4 from Peter Belm   2010-03-13 06:38:23 ---
Created an attachment (id=26780)
 --> (http://bugs.winehq.org/attachment.cgi?id=26780)
Full debug trace

[Bug 22020] Page Fault in wine_utf8_wcstombs when running ToonTalk

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

Paul Vriens  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |[e-mail]

--- Comment #5 from Paul Vriens   2010-03-13 10:03:47 ---
Looks like the length calculation in bstr_to_utf8() is not correct. Changing
SysStringLen() to lstrlenW() makes the game start (well at least doesn't crash)
but that's obviously not correct.

To my (I admit limited) knowledge this means that the string was not allocated
by one of Sys*Alloc ones.

[Bug 22020] Page Fault in wine_utf8_wcstombs when running ToonTalk

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

--- Comment #6 from Vitaliy Margolen   2010-03-13 11:01:21 ---
(In reply to comment #5)
> Looks like the length calculation in bstr_to_utf8() is not correct.

Yup - it does not include terminating \0 character which is required for utf8
strings.

[Bug 22020] Page Fault in wine_utf8_wcstombs when running ToonTalk

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

--- Comment #7 from Paul Vriens   2010-03-13 12:39:04 ---
What I meant is that SysStringLen makes use of length calculation by using the
DWORD in front of the real string (BSTR).

This only works if the string was allocated with SysAlloc* and SysReAlloc*.

The string that is passed around is not allocated with any of these functions
from the looks of it (and we get a huge size of: srclen=0x22a9aa)

[Bug 22020] Page Fault in wine_utf8_wcstombs when running ToonTalk

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

--- Comment #8 from Nikolay Sivov   2010-03-13 12:57:33 ---
(In reply to comment #7)
> The string that is passed around is not allocated with any of these functions
> from the looks of it (and we get a huge size of: srclen=0x22a9aa)

So application passes improper BSTR to domdoc_loadXML?

[Bug 22020] Page Fault in wine_utf8_wcstombs when running ToonTalk

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

--- Comment #9 from Peter Belm   2010-03-13 13:01:24 ---
(In reply to comment #8)
> (In reply to comment #7)
> > The string that is passed around is not allocated with any of these functions
> > from the looks of it (and we get a huge size of: srclen=0x22a9aa)
>
> So application passes improper BSTR to domdoc_loadXML?

If this is the case then it's behaviour domdoc_loadXML supports (or at least
tolerates), since the game works fine in Windows XP SP2,

[Bug 22020] Page Fault in wine_utf8_wcstombs when running ToonTalk

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

--- Comment #10 from Paul Vriens   2010-03-13 13:47:57 ---
As said, I'm by no means an expert but googling for 'loadXML' shows several
examples with 'normal' strings. This could of course also mean that loadXML
figures out whether it's passed a BSTR of WCHAR.

(I guess we need some tests).

[Bug 22020] Page Fault in wine_utf8_wcstombs when running ToonTalk

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

Nikolay Sivov  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |[e-mail]

--- Comment #11 from Nikolay Sivov   2010-03-13 13:49:01 ---
(In reply to comment #9)
> If this is the case then it's behaviour domdoc_loadXML supports (or at least
> tolerates), since the game works fine in Windows XP SP2,

Let's try to check where this string comes from. Attach +tid,+ole,+relay
please.

[Bug 22020] Page Fault in wine_utf8_wcstombs when running ToonTalk

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

--- Comment #12 from Nikolay Sivov   2010-03-13 13:54:57 ---
(In reply to comment #10)
> As said, I'm by no means an expert but googling for 'loadXML' shows several
> examples with 'normal' strings. This could of course also mean that loadXML
> figures out whether it's passed a BSTR of WCHAR.

This most likely means that it doesn't use length stored if ptr-- position.
Actually I don't think we should use it either, cause it's safe to pass -1 to
WideCharToMultiByte cause BSTR are always null terminated. WideCharToMultiByte
needs a full string scan for both calls so I don't think we are going to have a
speed change after that.

[Bug 22020] Page Fault in wine_utf8_wcstombs when running ToonTalk

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

--- Comment #13 from Paul Vriens   2010-03-13 13:56:45 ---
Created an attachment (id=26786)
 --> (http://bugs.winehq.org/attachment.cgi?id=26786)
Some extra tests

These new tests succeed on XP but not on Wine (obviously)

[Bug 22020] Page Fault in wine_utf8_wcstombs when running ToonTalk

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

--- Comment #14 from Nikolay Sivov   2010-03-13 14:15:33 ---
(In reply to comment #13)
> Created an attachment (id=26786)

--> (http://bugs.winehq.org/attachment.cgi?id=26786) [details]
> Some extra tests
>
> These new tests succeed on XP but not on Wine (obviously)

Could you please try another two cases:
- 0x1 pointer to check it crashes on XP and doesn't have exception handler;
- an invalid "BSTR" - WCHAR a[2 + <textlength>]. Where *(DWORD*)a = <more then
text length>.

Also it's interesting what does native in case of nulls in a middle of passed
string. Let's say does it parse after nulls if BSTR length is valid?

[Bug 22020] Page Fault in wine_utf8_wcstombs when running ToonTalk

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

Nikolay Sivov  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |[e-mail]

--- Comment #15 from Nikolay Sivov   2010-03-13 15:20:33 ---
*** Bug 18476 has been marked as a duplicate of this bug. ***

[Bug 22020] Page Fault in wine_utf8_wcstombs when running ToonTalk

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

--- Comment #16 from Peter Belm   2010-03-13 17:31:37 ---
(In reply to comment #11)
> (In reply to comment #9)
> > If this is the case then it's behaviour domdoc_loadXML supports (or at least
> > tolerates), since the game works fine in Windows XP SP2,
>
> Let's try to check where this string comes from. Attach +tid,+ole,+relay
> please.

I've done this, but the resulting log is over 8MB, should I attach the whole
thing or should I look for something in particular?

[Bug 22020] Page Fault in wine_utf8_wcstombs when running ToonTalk

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

--- Comment #17 from Dmitry Timoshkov   2010-03-13 23:32:55 ---
How large is the log after 'bzip2 -9' ?

[Bug 22020] Page Fault in wine_utf8_wcstombs when running ToonTalk

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

--- Comment #18 from Peter Belm   2010-03-14 06:14:51 ---
Created an attachment (id=26795)
 --> (http://bugs.winehq.org/attachment.cgi?id=26795)
Log with +tid,+ole,+relay (compressed)

[Bug 22020] Page Fault in wine_utf8_wcstombs when running ToonTalk

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

--- Comment #19 from Nikolay Sivov   2010-03-14 07:27:31 ---
(In reply to comment #18)
> Created an attachment (id=26795)

--> (http://bugs.winehq.org/attachment.cgi?id=26795) [details]
> Log with +tid,+ole,+relay (compressed)

Thanks. Here it is:

---
0009:Call ntdll.RtlAllocateHeap(01c60000,00000000,0000024e) ret=0054f507
0009:Ret  ntdll.RtlAllocateHeap() retval=01c6ad60 ret=0054f507
...
0009:Call oleaut32.SysStringLen(01c6ad60 ...) ret=20022c11
0009:Ret  oleaut32.SysStringLen() retval=0122a9aa ret=20022c11
...
0009:Call KERNEL32.WideCharToMultiByte(0000fde9,00000000,01c6ad60
L"...stripped...",0122a9aa,00000000,00000000,00000000,00000000) ret=20022c59 
---

So all we have to do now is to test ::loadXML with BSTR with nulls inside. If
doesn't go after first null - a perfect case, I'll patch it to remove this
length use.

[Bug 22020] Page Fault in wine_utf8_wcstombs when running ToonTalk

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

--- Comment #20 from Paul Vriens   2010-03-14 07:51:32 ---
Some tests:

Normal WCHAR : succeeds on XP
r = IXMLDOMDocument_loadXML( doc, szComplete1, &b );

Invalid BSTR pointer : crashes on XP
r = IXMLDOMDocument_loadXML( doc, 0x1, &b );

Invalid BSTR (Nikolay, is this what you meant?): fails on XP

WCHAR newstring[1024];
lstrcpyW(newstring + 2, szComplete1);
*(DWORD*)newstring = 0xffff;
r = IXMLDOMDocument_loadXML( doc, newstring, &b );

[Bug 22020] Page Fault in wine_utf8_wcstombs when running ToonTalk

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

--- Comment #21 from Nikolay Sivov   2010-03-14 07:57:44 ---
(In reply to comment #20)
> Some tests:
>
> Normal WCHAR : succeeds on XP
> r = IXMLDOMDocument_loadXML( doc, szComplete1, &b );
>
> Invalid BSTR pointer : crashes on XP
> r = IXMLDOMDocument_loadXML( doc, 0x1, &b );

Good.
> Invalid BSTR (Nikolay, is this what you meant?): fails on XP
>
> WCHAR newstring[1024];
> lstrcpyW(newstring + 2, szComplete1);
> *(DWORD*)newstring = 0xffff;
> r = IXMLDOMDocument_loadXML( doc, newstring, &b );

Almost, but you should pass newstring+2 to loadXML.

[Bug 22020] Page Fault in wine_utf8_wcstombs when running ToonTalk

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

--- Comment #22 from Nikolay Sivov   2010-03-14 08:04:50 ---
(In reply to comment #21)
> Almost, but you should pass newstring+2 to loadXML.

&newstring[2] to be exact.

About nulls inside string - we could use SysAllocStringLen to create with any
number of nulls inside.

[Bug 22020] Page Fault in wine_utf8_wcstombs when running ToonTalk

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

Paul Vriens  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #26786|0                           |1
        is obsolete|                            |

--- Comment #23 from Paul Vriens   2010-03-14 14:29:31 ---
Created an attachment (id=26805)
 --> (http://bugs.winehq.org/attachment.cgi?id=26805)
Extra tests

Extra tests that succeed on XP. The SysStringLen tests was just to prove that
it uses the DWORD regardless of the stringsize.

[Bug 22020] Page Fault in wine_utf8_wcstombs when running ToonTalk

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

Nikolay Sivov  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
     Ever Confirmed|0                           |1

--- Comment #24 from Nikolay Sivov   2010-03-14 14:49:10 ---
Ok. I also tested with bogus BSTR length and nulls inside data - parser doesn't
eat it stopping on first null just like on LPWSTR.

I think it's enough for it, will send a patch shortly. Thanks, Paul.

[Bug 22020] Page Fault in wine_utf8_wcstombs when running ToonTalk

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

--- Comment #25 from Nikolay Sivov   2010-03-14 15:11:35 ---
Patch sent:

http://www.winehq.org/pipermail/wine-patches/2010-March/085773.html

[Bug 22020] Page Fault in wine_utf8_wcstombs when running ToonTalk/AvrStudio

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

André H.  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |[e-mail]
            Summary|Page Fault in               |Page Fault in
                   |wine_utf8_wcstombs when     |wine_utf8_wcstombs when
                   |running ToonTalk            |running ToonTalk/AvrStudio

[Bug 22020] Page Fault in wine_utf8_wcstombs when running ToonTalk/AvrStudio

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

Nikolay Sivov  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|                            |FIXED

--- Comment #26 from Nikolay Sivov   2010-03-15 12:54:51 ---
Fixed by commit 2060d80d24980b1c85ca144b2c43c2b5cbbc7ec3.

[Bug 22020] Page Fault in wine_utf8_wcstombs when running ToonTalk/AvrStudio

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

Alexandre Julliard  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|RESOLVED                    |CLOSED

--- Comment #27 from Alexandre Julliard   2010-03-19 14:11:07 ---
Closing bugs fixed in 1.1.41.

[Bug 22020] Page Fault in wine_utf8_wcstombs when running ToonTalk/AvrStudio

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

André H.  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
      Fixed by SHA1|                            |2060d80d24980b1c85ca144b2c4
                   |                            |3c2b5cbbc7ec3