winrar password box

[abwillis]

winrar requires GetComboBoxInfo for it password box, I cannot test this code as with the GCC/KMK changeover Odin will crash before getting that far:

Index: include/win/winuser.h
===================================================================
--- include/win/winuser.h	(revision 21924)
+++ include/win/winuser.h	(working copy)
@@ -1922,6 +1922,7 @@
 #define CB_GETDROPPEDWIDTH       0x015f
 #define CB_SETDROPPEDWIDTH       0x0160
 #define CB_INITSTORAGE           0x0161
+#define CB_GETCOMBOBOXINFO       0x0164
 
 /* Combo box notification codes */
 #define CBN_ERRSPACE        (-1)
@@ -3355,9 +3356,9 @@
 #endif /* defined(_WINGDI_) && !defined(NOGDI) */
 
 HKL         WINAPI ActivateKeyboardLayout(HKL,UINT);
-LONG        WINAPI BroadcastSystemMessageA(DWORD,LPDWORD,UINT,WPARAM,LPARAM); 
-LONG        WINAPI BroadcastSystemMessageW(DWORD,LPDWORD,UINT,WPARAM,LPARAM); 
-#define     BroadcastSystemMessage WINELIB_NAME_AW(BroadcastSystemMessage) 
+LONG        WINAPI BroadcastSystemMessageA(DWORD,LPDWORD,UINT,WPARAM,LPARAM);
+LONG        WINAPI BroadcastSystemMessageW(DWORD,LPDWORD,UINT,WPARAM,LPARAM);
+#define     BroadcastSystemMessage WINELIB_NAME_AW(BroadcastSystemMessage)
 WORD        WINAPI CascadeWindows(HWND, UINT, const LPRECT, UINT, const HWND *);
 BOOL        WINAPI CascadeChildWindows(HWND, DWORD);
 INT       WINAPI CopyAcceleratorTableA(HACCEL,LPACCEL,INT);
Index: src/user32/combo.c
===================================================================
--- src/user32/combo.c	(revision 21924)
+++ src/user32/combo.c	(working copy)
@@ -1885,7 +1885,24 @@
    }
 }
 
+static LRESULT COMBO_GetComboBoxInfo(const HEADCOMBO *lphc, COMBOBOXINFO *pcbi)
+{
+    if (!pcbi || (pcbi->cbSize < sizeof(COMBOBOXINFO)))
+        return FALSE;
 
+    pcbi->rcItem = lphc->textRect;
+    pcbi->rcButton = lphc->buttonRect;
+    pcbi->stateButton = 0;
+    if (lphc->wState & CBF_BUTTONDOWN)
+        pcbi->stateButton |= STATE_SYSTEM_PRESSED;
+    if (IsRectEmpty(&lphc->buttonRect))
+        pcbi->stateButton |= STATE_SYSTEM_INVISIBLE;
+    pcbi->hwndCombo = lphc->self;
+    pcbi->hwndItem = lphc->hWndEdit;
+    pcbi->hwndList = lphc->hWndLBox;
+    return TRUE;
+}
+
 /***********************************************************************
  *           ComboWndProc_common
  *
@@ -2278,7 +2295,8 @@
     case CB_GETEXTENDEDUI16:
     case CB_GETEXTENDEDUI:
         return  (lphc->wState & CBF_EUI) ? TRUE : FALSE;
-
+    case CB_GETCOMBOBOXINFO:
+        return COMBO_GetComboBoxInfo(lphc, (COMBOBOXINFO *)lParam);
     default:
         if (message >= WM_USER)
             WARN("unknown msg WM_USER+%04x wp=%04x lp=%08lx\n",
@@ -2309,3 +2327,13 @@
     if (!IsWindow(hwnd)) return 0;
     return ComboWndProc_common( hwnd, message, wParam, lParam, TRUE );
 }
+
+/*************************************************************************
+ *           GetComboBoxInfo   (USER32.@)
+ */
+BOOL WINAPI GetComboBoxInfo(HWND hwndCombo,      /* [in] handle to combo box */
+                PCOMBOBOXINFO pcbi   /* [in/out] combo box information */)
+{
+    TRACE("(%p, %p)\n", hwndCombo, pcbi);
+    return SendMessageW(hwndCombo, CB_GETCOMBOBOXINFO, 0, (LPARAM)pcbi);
+}
Index: src/user32/user32.def
===================================================================
--- src/user32/user32.def	(revision 21924)
+++ src/user32/user32.def	(working copy)
@@ -769,3 +769,6 @@
 
     "_RunOnAuxThread@20"                                         @2048 NONAME
     "_RunOnAuxThreadAndWait@24"                                  @2049 NONAME
+
+; New but not used by Flash
+    "_GetComboBoxInfo@8"                                         @2050

[dmik]

Can you give more information about the winrar crash?

[abwillis]

If I try to open an archive it will just close, I haven't had a chance to build debug yet to get logs to figure out where/why (no error or popuplog.os2 entry).

[abwillis]

One other piece that may be related or a sign of another issue, menus will normally crash in winrar as well. After a fresh reboot I can click on a menu item and have it drop down but then selecting something from it will crash... after that it will crash just clicking on the menu item. i.e. Click Help the first time it drops down, click on about and it closes. Relaunch winrar and click Help and it closes. After a reboot I can again click the Help without a crash.

As a point of reference, I just tested again after rebuilding with the Flash fix with the same results.

[dmik]

I confirm the crash. Happens somewhere in _umalloc (0001:0000A629 in LIBC064.DLL). The call stack is like this:

_um_lump_set_size
_um_lump_make_free
...
_umalloc
...
OS2Heap::Alloc
HeapAlloc@12
notify_dispinfoT
LISTVIEW_GetItemT
LISTVIEW_DrawItem

[dmik]

So far, it looks like there is a problem with the umalloc API implementation in LIBC064. Needs a deeper investigation. The problems seems to show up after the heap gets growing -- if I increase the initial size (which is just 0x4000), WinRar stays longer w/o crashing (but eventually crashes at the very same place later).

This may be hard to debug so I'm postponing it for now (in favor of the Flash plugin). But since any application that involves frequent heap allocations will theoretically suffer from this issue, we should address it soon.

[dmik]

JFTR, GetComboBoxInfo was added in r21949.

Still debugging the umalloc issue.

[dmik]

I did several standalone tests for the umalloc API and can't find any problems using these tests.

This may mean that it crashes because the application corrupts the heap memory (i.e. writes beyond the bounds) which in turn corrupts control headers preceding heap memory blocks. At least, all crashes are failed assertions so far which is also an indication. I will check this path.

[abwillis]

Is the umalloc issue then something that has to be fixed in klibc or something that would be fixed in Odin?

[dmik]

It's definitely the heap corruption. I added some debug printing and now see that at a certain point the heap gets corrupted. Investigating.

[dmik]

Okay, I found what's wrong. Odin doesn't align the width of Win32 bitmaps in bytes to the 4-byte boundary, while OS/2 requires it to be such. For example, for the 4-bit color depth bitmap 20x20px, the Win32 bitmap header specifies 10 bytes per scan-line, while OS/2 expects it to take 12 bytes.

As a result, when bitmap bits are queried by GpiQueryBitmapBits() to an array calculated using the Win32 header, the data doesn't fit it and writing them beyond the bounds will corrupt the heap (where the data array is allocated from). Note that this only happens for bitmaps of certain size and depth combinations (those not aligned on the 4-byte boundary on their own). Also note that it was not seen in VAC because it seems to have a different internal heap layout so that getting beyond the bounds by a few bytes didn't corrupt it.

The next question is what is the correct alignment for Win32 bitmaps. If it is not 4-byte, then we will have to use temporary buffers to overcome this alignment difference when querying the bitmap bits.

[dmik]

Windows actually seems to use both alignments. ​http://msdn.microsoft.com/en-us/library/dd183375%28v=vs.85%29.aspx says that scan lines are aligned on a LONG (4-byte) boundary (this affects GetDIBits and others), while ​http://msdn.microsoft.com/en-us/library/dd183371%28v=vs.85%29.aspx says that it should be at least aligned on a word (2-byte) boundary (this seems to relate to CreateBitmap). This is a bit confusing.

In r21951, I fixed this particular place of the problem (CreateIconIndirect()/CreateIconFromResource()/LoadImage()/etc.) by ensuring that the arrays passed to GetDIBits() are LONG-aligned. WinRAR doesn't crash when navigating through the menus any longer.

[dmik]

Note that the attempt to call a WinRAR dialog with the password box (using Ctrl-P) crashes the application on its own. But this seems to be completely unrelated to the bitmap data alignment problem I fixed above. Needs more investigation.

[dmik]

BTW, in r21952, I committed a hack that fixes GetFullPathName() that would return garbage and would not let e.g. enter the archive file in WinRAR and unpack it.

[dmik]

I checked the new crash issue; it seems that it happens before the password combobox is about to be used. The crash is in the Win32 code (WinRAR.EXE, address 00FF:0004F51A). Looks like a function is passed a pointer to a structure whose member is NULL and this member (which is also a pointer to a structure) is then dereferenced for storing some value at offset 0B and since it's NULL the program crashes.

The recent activity before the crash is an attempt to instantiate some missing COM object (access to an InprocServer32 key of some CLSID, {00BB2763-6A77-11D0-A535-00C04FD7D062}) and then an attempt to read the contents of the Software\WInRAR\Passwords key.

So, as a guess, WinRAR gets unexpected input which results into a NULL pointer which is not checked.

Anyway, I'm moving this out of this milestone since it is not really critical for it. Andy, if you find anything particular in your tests, please inform here.

[dmik]

JFYI, the above CLSID is the "Microsoft AutoComplete" service. Looks like WinRAR wants it hard.

[abwillis]

Winrar 4.01 and 4.1b5 crashes, though Winrar 3.93 does work now.