[Bug 19839] New: Access violation when closing ImgBurn 2.5.0.0 if its 'Disc Layout Editor' window has ever been opened.

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

           Summary: Access violation when closing ImgBurn 2.5.0.0 if its
                    'Disc Layout Editor' window has ever been opened.
           Product: Wine
           Version: 1.1.28
          Platform: PC
               URL: http://www.imgburn.com
        OS/Version: Linux
            Status: UNCONFIRMED
          Severity: normal
          Priority: P2
         Component: -unknown
        AssignedTo: wine-bugs(a)winehq.org
        ReportedBy: lightning_uk(a)imgburn.com


v2.5.0.0 of ImgBurn has a new Advanced build mode interface known as the 'Disc
Layout Editor'.

If you open the window at all (as in, 'full stop' and not even doing anything
with it) and then close the program, it'll crash with an access violation.

Open the program and set the 'Mode' to 'Build', set the 'Input' to 'Advanced'
 and then click the big 'Show Disc Layout Editor' button. Once you've done that
and you attempt to close the program, it'll crash.

I'm unable to reproduce this issue on any real installation of Windows
(from Windows 95 -> Windows 7) so I can only assume it's something in Wine
(sorry!).

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

[Bug 19839] Access violation when closing ImgBurn 2.5.0.0 if its 'Disc Layout Editor' window has ever been opened.

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


Juan Lang <juan_lang(a)yahoo.com> changed:

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




--- Comment #1 from Juan Lang <juan_lang(a)yahoo.com>  2009-08-25 12:55:40 ---
Confirming.  Thanks for the detailed bug report.

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

[Bug 19839] Access violation when closing ImgBurn 2.5.0.0 if its 'Disc Layout Editor' window has ever been opened.

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


Juan Lang <juan_lang(a)yahoo.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
          Component|-unknown                    |comctl32




--- Comment #2 from Juan Lang <juan_lang(a)yahoo.com>  2009-08-25 13:03:59 ---
I installed a native common controls library using 'winetricks cc580', and the
access violation goes away.  Moving component to comctl32.

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

[Bug 19839] Access violation when closing ImgBurn 2.5.0.0 if its 'Disc Layout Editor' window has ever been opened.

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





--- Comment #3 from Juan Lang <juan_lang(a)yahoo.com>  2009-08-25 16:21:27 ---
> From a +relay,+seh log:

0009:Ret  window proc 0x1de06cb
(hwnd=0x201a8,msg=WM_GETTEXT,wp=00000001,lp=0032ef54) retval=ffffffff
0009:Ret  user32.SendMessageA() retval=7fffffff ret=0080e46c
trace:seh:raise_exception code=c0000005 flags=0 addr=0x7da1dd ip=007da1dd
tid=0009

The return value from WM_GETTEXT looks a little funny: it's returning -1.  MSDN
says the return value is the number of characters copied, excluding the NULL
terminator.  A little closer look reveals it's the statusbar control that's
returning -1.  I'll attach a patch in a sec, though it'll need a test to
confirm it.

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

[Bug 19839] Access violation when closing ImgBurn 2.5.0.0 if its 'Disc Layout Editor' window has ever been opened.

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





--- Comment #4 from Juan Lang <juan_lang(a)yahoo.com>  2009-08-25 16:25:19 ---
Created an attachment (id=23250)
 --> (http://bugs.winehq.org/attachment.cgi?id=23250)
Patch: Return the number of characters copied in WM_GETTEXT even if the buffer
is too small

This fixes the crash for me, although it deserves a test case to confirm it.

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

[Bug 19839] Access violation when closing ImgBurn 2.5.0.0 if its 'Disc Layout Editor' window has ever been opened.

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


Juan Lang <juan_lang(a)yahoo.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |download, patch




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

[Bug 19839] Access violation when closing ImgBurn 2.5.0.0 if its 'Disc Layout Editor' window has ever been opened.

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


Anastasius Focht <focht(a)gmx.net> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |focht(a)gmx.net




--- Comment #5 from Anastasius Focht <focht(a)gmx.net>  2009-08-25 17:03:54 ---
Hello,

looks like dupe of bug 13411

Regards

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

[Bug 19839] Access violation when closing ImgBurn 2.5.0.0 if its 'Disc Layout Editor' window has ever been opened.

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


Juan Lang <juan_lang(a)yahoo.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |juan_lang(a)yahoo.com




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

[Bug 19839] Access violation when closing ImgBurn 2.5.0.0 if its 'Disc Layout Editor' window has ever been opened.

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





--- Comment #6 from LIGHTNING UK! <lightning_uk(a)imgburn.com>  2009-08-25 19:26:48 ---
I haven't looked at anything except your patch (i.e no actual source code) but
wouldn't you simply have to 'return 0;' rather than 'return -1' ? You can't
copy '-1' characters into the buffer so '-1' isn't a valid return value
(according to MSDN anyway - and as you pointed out earlier).

The bit in your patch where you memcpy looks like it could cause its own access
violation if 'size' is larger than the length of 'infoPtr->parts[0].text' - no
?
> > memcpy (buf, infoPtr->parts[0].text, (size - 1) * sizeof(WCHAR));

and along the same line, why is this being used (again, I've NOT looked at the
source code, just what I can see in your patch!)...
> > strcpyW (buf, infoPtr->parts[0].text);

...rather that one that takes into account the actual max length of 'buf' (as
specified by the 'size' parameter) - strLcpyW or whatever it would be called.

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

[Bug 19839] Access violation when closing ImgBurn 2.5.0.0 if its 'Disc Layout Editor' window has ever been opened.

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





--- Comment #7 from Juan Lang <juan_lang(a)yahoo.com>  2009-08-25 19:35:48 ---
(In reply to comment #6)
> I haven't looked at anything except your patch (i.e no actual source code) but
> wouldn't you simply have to 'return 0;' rather than 'return -1' ?

No.  I suppose I could have tested that as well, that is, what should it return
when the buffer is 2 characters in length rather than 1, and the status bar
text is longer than 1 character?  I followed MSDN here.
> The bit in your patch where you memcpy looks like it could cause its own access
> violation if 'size' is larger than the length of 'infoPtr->parts[0].text' - no
> ?

This condition is impossible due to the if clause:
    len = strlenW (infoPtr->parts[0].text);

    if (size > len) {
(snip)
    else {

The else branch, which you're seeing, only gets run if size is NOT greater than
the length of infoPtr->parts[0].text.

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

[Bug 19839] Access violation when closing ImgBurn 2.5.0.0 if its 'Disc Layout Editor' window has ever been opened.

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





--- Comment #8 from LIGHTNING UK! <lightning_uk(a)imgburn.com>  2009-08-25 19:48:11 ---
(In reply to comment #7)
> No.  I suppose I could have tested that as well, that is, what should it return
> when the buffer is 2 characters in length rather than 1, and the status bar
> text is longer than 1 character?  I followed MSDN here.

If size (wParam in WM_GETTEXT message) is 2 then it only has room for 1 actual
character and the terminating null terminator so the return value should be 1.

This is regardless of how long 'infoPtr->parts[0].text' is because only 1
character can be copied into buf - the terminating null isn't counted in the
return value.

I'm sure we're reading MSDN the same way :)
> This condition is impossible due to the if clause:
>     len = strlenW (infoPtr->parts[0].text);
>     if (size > len) {
> (snip)
>     else {
> The else branch, which you're seeing, only gets run if size is NOT greater than
> the length of infoPtr->parts[0].text.

Ah ok, that makes sense with a little more context. :)

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

[Bug 19839] Access violation when closing ImgBurn 2.5.0.0 if its 'Disc Layout Editor' window has ever been opened.

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





--- Comment #9 from Juan Lang <juan_lang(a)yahoo.com>  2009-08-25 21:18:26 ---
(In reply to comment #8)
> If size (wParam in WM_GETTEXT message) is 2 then it only has room for 1 actual
> character and the terminating null terminator so the return value should be 1.

Yes, precisely.  So, since size is _not_ greater than len (the length of
infoPtr->parts[0].text), the else branch is taken, and size - 1 characters are
copied to the output buffer, and size - 1 is returned.  Make sense?

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

[Bug 19839] Access violation when closing ImgBurn 2.5.0.0 if its 'Disc Layout Editor' window has ever been opened.

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





--- Comment #10 from LIGHTNING UK! <lightning_uk(a)imgburn.com>  2009-08-26 04:10:40 ---
(In reply to comment #9)
> Yes, precisely.  So, since size is _not_ greater than len (the length of
> infoPtr->parts[0].text), the else branch is taken, and size - 1 characters are
> copied to the output buffer, and size - 1 is returned.  Make sense?

Yup, so long as the potential case where size == 0 is handled somewhere else
higher up. If size < 2, the return value should in theory always be 0.

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

[Bug 19839] Access violation when closing ImgBurn 2.5.0.0 if its 'Disc Layout Editor' window has ever been opened.

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





--- Comment #11 from LIGHTNING UK! <lightning_uk(a)imgburn.com>  2009-08-26 04:51:10 ---
Oops, seems that if size is 0, windows actually returns the number of
characters that *would* be copied into the buffer *if* there was sufficient
allocation.

So this should work?

    len = strlenW (infoPtr->parts[0].text);

    if(size == 0)
    {
      return len;
    }
    else if(size > len)
    {
      strcpyW (buf, infoPtr->parts[0].text);
      return len;
    }
    else
    {
      memcpy (buf, infoPtr->parts[0].text, (size - 1) * sizeof(WCHAR));
      buf[size - 1] = 0; // This has to be 'size - 1' rather than just 'size'
or you'll get an access overrun?
      return size - 1;
    }

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

[Bug 19839] Access violation when closing ImgBurn 2.5.0.0 if its 'Disc Layout Editor' window has ever been opened.

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





--- Comment #12 from Juan Lang <juan_lang(a)yahoo.com>  2009-08-26 10:51:04 ---
(In reply to comment #11)
> Oops, seems that if size is 0, windows actually returns the number of
> characters that *would* be copied into the buffer *if* there was sufficient
> allocation.

Thanks, I hadn't tested that.  Right you are.  I sent an updated patch that
addresses this case.
> memcpy (buf, infoPtr->parts[0].text, (size - 1) * sizeof(WCHAR));
>       buf[size - 1] = 0; // This has to be 'size - 1' rather than just 'size'
> or you'll get an access overrun?

Heh.  Right.

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

[Bug 19839] Access violation when closing ImgBurn 2.5.0.0 if its 'Disc Layout Editor' window has ever been opened.

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


Juan Lang <juan_lang(a)yahoo.com> changed:

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




--- Comment #13 from Juan Lang <juan_lang(a)yahoo.com>  2009-08-27 10:48:29 ---
Fixed by commit 390a248e0649cf5cc4c4e8a476f0747bc204c3ba.  Thanks for your help
:)

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

[Bug 19839] Access violation when closing ImgBurn 2.5.0.0 if its 'Disc Layout Editor' window has ever been opened.

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


Alexandre Julliard <julliard(a)winehq.org> changed:

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




--- Comment #14 from Alexandre Julliard <julliard(a)winehq.org>  2009-09-02 14:31:34 ---
Closing bugs fixed in 1.1.29.

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

[Bug 19839] Access violation when closing ImgBurn 2.5.0.0 if its 'Disc Layout Editor' window has ever been opened.

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

Nikolay Sivov <bunglehead(a)gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
      Fixed by SHA1|                            |390a248e0649cf5cc4c4e8a476f
                   |                            |0747bc204c3ba

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