[PATCH 0/2] MR8346: wininet: Fix 2 out-of-bound problems in url cache hash calculation

[Yuxuan Shui (＠yshui)]

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/8346

[PATCH 1/2] wininet: Fix handling of empty strings in urlcache_hash_key.

[Yuxuan Shui]

From: Yuxuan Shui <yshui(a)codeweavers.com>

---
 dlls/wininet/urlcache.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/dlls/wininet/urlcache.c b/dlls/wininet/urlcache.c
index 7975071ce91..366fa0b6bea 100644
--- a/dlls/wininet/urlcache.c
+++ b/dlls/wininet/urlcache.c
@@ -1461,11 +1461,12 @@ static DWORD urlcache_hash_key(LPCSTR lpszKey)
     for (i = 0; i < ARRAY_SIZE(key); i++)
         key[i] = lookupTable[(*lpszKey + i) & 0xFF];
 
-    for (lpszKey++; *lpszKey; lpszKey++)
-    {
-        for (i = 0; i < ARRAY_SIZE(key); i++)
-            key[i] = lookupTable[*lpszKey ^ key[i]];
-    }
+    if (*lpszKey)
+        for (lpszKey++; *lpszKey; lpszKey++)
+        {
+            for (i = 0; i < ARRAY_SIZE(key); i++)
+                key[i] = lookupTable[*lpszKey ^ key[i]];
+        }
 
     return *(DWORD *)key;
 }
-- 
GitLab


https://gitlab.winehq.org/wine/wine/-/merge_requests/8346

[PATCH 2/2] wininet: Use BYTE instead of char for hash calculation.

[Yuxuan Shui]

From: Yuxuan Shui <yshui(a)codeweavers.com>

Since LPCSTR is signed, (*lpszKey ^ key[i]) could be negative, which means
accessing the lookupTable with it will underflow.
---
 dlls/wininet/urlcache.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/dlls/wininet/urlcache.c b/dlls/wininet/urlcache.c
index 366fa0b6bea..3d8177b2872 100644
--- a/dlls/wininet/urlcache.c
+++ b/dlls/wininet/urlcache.c
@@ -1415,7 +1415,7 @@ static DWORD urlcache_set_entry_info(entry_url *url_entry, const INTERNET_CACHE_
  *    hash key for the string
  *
  */
-static DWORD urlcache_hash_key(LPCSTR lpszKey)
+static DWORD urlcache_hash_key(const BYTE *lpszKey)
 {
     /* NOTE: this uses the same lookup table as SHLWAPI.UrlHash{A,W}
      * but the algorithm and result are not the same!
@@ -1492,7 +1492,7 @@ static BOOL urlcache_find_hash_entry(const urlcache_header *pHeader, LPCSTR lpsz
      *  there can be multiple hash tables in the file and the offset to
      *  the next one is stored in the header of the hash table
      */
-    DWORD key = urlcache_hash_key(lpszUrl);
+    DWORD key = urlcache_hash_key((const BYTE *)lpszUrl);
     DWORD offset = (key & (HASHTABLE_NUM_ENTRIES-1)) * HASHTABLE_BLOCKSIZE;
     entry_hash_table* pHashEntry;
     DWORD id = 0;
@@ -1581,7 +1581,7 @@ static DWORD urlcache_hash_entry_create(urlcache_header *pHeader, LPCSTR lpszUrl
 {
     /* see urlcache_find_hash_entry for structure of hash tables */
 
-    DWORD key = urlcache_hash_key(lpszUrl);
+    DWORD key = urlcache_hash_key((const BYTE *)lpszUrl);
     DWORD offset = (key & (HASHTABLE_NUM_ENTRIES-1)) * HASHTABLE_BLOCKSIZE;
     entry_hash_table* pHashEntry, *pHashPrev = NULL;
     DWORD id = 0;
-- 
GitLab

https://gitlab.winehq.org/wine/wine/-/merge_requests/8346

[Jacek Caban (＠jacek)]

Jacek Caban (@jacek) commented about dlls/wininet/urlcache.c:
> *  there can be multiple hash tables in the file and the offset to
>       *  the next one is stored in the header of the hash table
>       */
> -    DWORD key = urlcache_hash_key(lpszUrl);
> +    DWORD key = urlcache_hash_key((const BYTE *)lpszUrl);

Not a big deal, but it might be cleaner to move the cast into `urlcache_hash_key` rather than repeating it at each call site.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/8346#note_107064