[PATCH 0/2] MR8346: wininet: Fix 2 out-of-bound problems in url cache hash calculation
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
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) 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
participants (3)
-
Jacek Caban (@jacek) -
Yuxuan Shui -
Yuxuan Shui (@yshui)