[PATCH] shell32: avoid memcmp result truncation (Coverity)

Marcus Meissner marcus at jet.franken.de
Sat Jul 7 04:52:23 CDT 2012


Hi,

memcmp() might return a full 32bit wide difference in optimized
memcmp cases, so we need to avoid truncating the upper 16 bits.

(mysql security flaw resulting from such a truncation:

https://community.rapid7.com/community/metasploit/blog/2012/06/11/cve-2012-2122-a-tragically-comedic-security-flaw-in-mysql
)

Ciao, Marcus
---
 dlls/shell32/recyclebin.c |    9 ++++++++-
 1 files changed, 8 insertions(+), 1 deletions(-)

diff --git a/dlls/shell32/recyclebin.c b/dlls/shell32/recyclebin.c
index 6b7d4aa..ad38daf 100644
--- a/dlls/shell32/recyclebin.c
+++ b/dlls/shell32/recyclebin.c
@@ -457,12 +457,19 @@ static HRESULT WINAPI RecycleBin_BindToStorage(IShellFolder2 *This, LPCITEMIDLIS
 static HRESULT WINAPI RecycleBin_CompareIDs(IShellFolder2 *iface, LPARAM lParam, LPCITEMIDLIST pidl1, LPCITEMIDLIST pidl2)
 {
     RecycleBin *This = impl_from_IShellFolder2(iface);
+    int ret;
 
     /* TODO */
     TRACE("(%p, %p, %p, %p)\n", This, (void *)lParam, pidl1, pidl2);
     if (pidl1->mkid.cb != pidl2->mkid.cb)
         return MAKE_HRESULT(SEVERITY_SUCCESS, 0, pidl1->mkid.cb - pidl2->mkid.cb);
-    return MAKE_HRESULT(SEVERITY_SUCCESS, 0, (unsigned short)memcmp(pidl1->mkid.abID, pidl2->mkid.abID, pidl1->mkid.cb));
+    /* Looks too complicated, but in optimized memcpy we might get
+     * a 32bit wide difference and would truncate it to 16 bit, so
+     * erronously returning equality. */
+    ret = memcmp(pidl1->mkid.abID, pidl2->mkid.abID, pidl1->mkid.cb);
+    if (ret < 0) ret = -1;
+    if (ret > 0) ret =  1;
+    return MAKE_HRESULT(SEVERITY_SUCCESS, 0, (unsigned short)ret);
 }
 
 static HRESULT WINAPI RecycleBin_CreateViewObject(IShellFolder2 *iface, HWND hwndOwner, REFIID riid, void **ppv)
-- 
1.7.3.4




More information about the wine-patches mailing list