Daniel Lehman : urlmon: Fix buffer overflow in parse_canonicalize.
Alexandre Julliard
julliard at winehq.org
Thu Nov 17 17:45:58 CST 2016
Module: wine
Branch: master
Commit: 22b4599b8a715cb6ec71351ed24795fce1530195
URL: http://source.winehq.org/git/wine.git/?a=commit;h=22b4599b8a715cb6ec71351ed24795fce1530195
Author: Daniel Lehman <dlehman at esri.com>
Date: Fri Nov 4 13:26:02 2016 -0700
urlmon: Fix buffer overflow in parse_canonicalize.
Signed-off-by: Daniel Lehman <dlehman at esri.com>
Signed-off-by: Jacek Caban <jacek at codeweavers.com>
Signed-off-by: Alexandre Julliard <julliard at 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;
}
More information about the wine-cvs
mailing list