Module: wine Branch: master Commit: 22b4599b8a715cb6ec71351ed24795fce1530195 URL: http://source.winehq.org/git/wine.git/?a=commit;h=22b4599b8a715cb6ec71351ed2...
Author: Daniel Lehman dlehman@esri.com Date: Fri Nov 4 13:26:02 2016 -0700
urlmon: Fix buffer overflow in parse_canonicalize.
Signed-off-by: Daniel Lehman dlehman@esri.com Signed-off-by: Jacek Caban jacek@codeweavers.com Signed-off-by: Alexandre Julliard julliard@winehq.org
---
dlls/urlmon/tests/uri.c | 34 ++++++++++++++++++++++++++++++++++ dlls/urlmon/uri.c | 38 +++++++++++++++++++++++--------------- 2 files changed, 57 insertions(+), 15 deletions(-)
diff --git a/dlls/urlmon/tests/uri.c b/dlls/urlmon/tests/uri.c index 5f98d4e..da5a739 100644 --- a/dlls/urlmon/tests/uri.c +++ b/dlls/urlmon/tests/uri.c @@ -7405,6 +7405,11 @@ static inline LPWSTR a2w(LPCSTR str) { return ret; }
+static inline void *heap_alloc(size_t len) +{ + return HeapAlloc(GetProcessHeap(), 0, len); +} + static inline BOOL heap_free(void* mem) { return HeapFree(GetProcessHeap(), 0, mem); } @@ -10403,7 +10408,9 @@ static void test_CoInternetParseIUri_InvalidArgs(void) { HRESULT hr; IUri *uri = NULL; WCHAR tmp[3]; + WCHAR *longurl, *copy; DWORD result = -1; + DWORD i, len;
hr = pCoInternetParseIUri(NULL, PARSE_CANONICALIZE, 0, tmp, 3, &result, 0); ok(hr == E_INVALIDARG, "Error: CoInternetParseIUri returned 0x%08x, expected 0x%08x.\n", @@ -10459,6 +10466,33 @@ static void test_CoInternetParseIUri_InvalidArgs(void) { expected_len, result); } if(uri) IUri_Release(uri); + + /* a long url that causes a crash on Wine */ + len = INTERNET_MAX_URL_LENGTH*2; + longurl = heap_alloc((len+1)*sizeof(WCHAR)); + memcpy(longurl, http_urlW, sizeof(http_urlW)); + for(i = sizeof(http_urlW)/sizeof(WCHAR)-1; i < len; i++) + longurl[i] = 'x'; + longurl[len] = 0; + + copy = heap_alloc((len+1)*sizeof(WCHAR)); + memcpy(copy, longurl, (len+1)*sizeof(WCHAR)); + + hr = pCreateUri(longurl, 0, 0, &uri); + ok(SUCCEEDED(hr), "Error: CreateUri returned 0x%08x.\n", hr); + if(SUCCEEDED(hr)) { + result = -1; + memset(longurl, 0xcc, len*sizeof(WCHAR)); + hr = pCoInternetParseIUri(uri, PARSE_CANONICALIZE, 0, longurl, len+1, &result, 0); + ok(SUCCEEDED(hr), "Error: CoInternetParseIUri returned 0x%08x.\n", hr); + ok(!lstrcmpW(longurl, copy), "Error: expected long url '%s' but was '%s'.\n", + wine_dbgstr_w(copy), wine_dbgstr_w(longurl)); + ok(result == len, "Error: Expected 'result' to be %d, but was %d instead.\n", + len, result); + } + heap_free(longurl); + heap_free(copy); + if(uri) IUri_Release(uri); }
static void test_CoInternetParseIUri(void) { diff --git a/dlls/urlmon/uri.c b/dlls/urlmon/uri.c index e6f844b..79b073e 100644 --- a/dlls/urlmon/uri.c +++ b/dlls/urlmon/uri.c @@ -6825,7 +6825,6 @@ static HRESULT parse_canonicalize(const Uri *uri, DWORD flags, LPWSTR output, const WCHAR *ptr = NULL; WCHAR *path = NULL; const WCHAR **pptr; - WCHAR buffer[INTERNET_MAX_URL_LENGTH+1]; DWORD len = 0; BOOL reduce_path;
@@ -6852,11 +6851,11 @@ static HRESULT parse_canonicalize(const Uri *uri, DWORD flags, LPWSTR output, * it later. */ if(reduce_path && !path && ptr == uri->canon_uri+uri->path_start) - path = buffer+len; + path = output+len;
/* Check if it's time to reduce the path. */ if(reduce_path && ptr == uri->canon_uri+uri->path_start+uri->path_len) { - DWORD current_path_len = (buffer+len) - path; + DWORD current_path_len = (output+len) - path; DWORD new_path_len = remove_dot_segments(path, current_path_len);
/* Update the current length. */ @@ -6868,7 +6867,9 @@ static HRESULT parse_canonicalize(const Uri *uri, DWORD flags, LPWSTR output, const WCHAR decoded = decode_pct_val(ptr); if(decoded) { if(allow_unescape && (flags & URL_UNESCAPE)) { - buffer[len++] = decoded; + if(len < output_len) + output[len] = decoded; + len++; ptr += 2; do_default_action = FALSE; } @@ -6876,48 +6877,55 @@ static HRESULT parse_canonicalize(const Uri *uri, DWORD flags, LPWSTR output,
/* See if %'s needed to encoded. */ if(do_default_action && (flags & URL_ESCAPE_PERCENT)) { - pct_encode_val(*ptr, buffer+len); + if(len + 3 < output_len) + pct_encode_val(*ptr, output+len); len += 3; do_default_action = FALSE; } } else if(*ptr == ' ') { if((flags & URL_ESCAPE_SPACES_ONLY) && !(flags & URL_ESCAPE_UNSAFE)) { - pct_encode_val(*ptr, buffer+len); + if(len + 3 < output_len) + pct_encode_val(*ptr, output+len); len += 3; do_default_action = FALSE; } } else if(!is_reserved(*ptr) && !is_unreserved(*ptr)) { if(flags & URL_ESCAPE_UNSAFE) { - pct_encode_val(*ptr, buffer+len); + if(len + 3 < output_len) + pct_encode_val(*ptr, output+len); len += 3; do_default_action = FALSE; } }
- if(do_default_action) - buffer[len++] = *ptr; + if(do_default_action) { + if(len < output_len) + output[len] = *ptr; + len++; + } }
/* Sometimes the path is the very last component of the IUri, so * see if the dot segments need to be reduced now. */ if(reduce_path && path) { - DWORD current_path_len = (buffer+len) - path; + DWORD current_path_len = (output+len) - path; DWORD new_path_len = remove_dot_segments(path, current_path_len);
/* Update the current length. */ len -= (current_path_len-new_path_len); }
- buffer[len++] = 0; + if(len < output_len) + output[len] = 0; + else + output[output_len-1] = 0;
/* The null terminator isn't included in the length. */ - *result_len = len-1; - if(len > output_len) + *result_len = len; + if(len >= output_len) return STRSAFE_E_INSUFFICIENT_BUFFER; - else - memcpy(output, buffer, len*sizeof(WCHAR));
return S_OK; }
-
Alexandre Julliard