[PATCH] winhttp: Escape untrusted URL paths.
Hans Leidekker
hans at codeweavers.com
Fri Aug 24 06:32:42 CDT 2018
Signed-off-by: Hans Leidekker <hans at codeweavers.com>
---
dlls/winhttp/request.c | 24 ++++++++++-----------
dlls/winhttp/session.c | 8 +++----
dlls/winhttp/tests/url.c | 1 +
dlls/winhttp/tests/winhttp.c | 40 ++++++++++++++++++++++++++++++++++
dlls/winhttp/url.c | 49 ++++++++++++++++++++++++------------------
dlls/winhttp/winhttp_private.h | 1 +
6 files changed, 86 insertions(+), 37 deletions(-)
diff --git a/dlls/winhttp/request.c b/dlls/winhttp/request.c
index 65d641f3cb..6f6de7c63d 100644
--- a/dlls/winhttp/request.c
+++ b/dlls/winhttp/request.c
@@ -2517,7 +2517,7 @@ static WCHAR *get_redirect_url( request_t *request, DWORD *len )
query_headers( request, WINHTTP_QUERY_LOCATION, NULL, NULL, &size, NULL );
if (get_last_error() != ERROR_INSUFFICIENT_BUFFER) return FALSE;
if (!(ret = heap_alloc( size ))) return NULL;
- *len = size / sizeof(WCHAR);
+ *len = size / sizeof(WCHAR) - 1;
if (query_headers( request, WINHTTP_QUERY_LOCATION, NULL, ret, &size, NULL )) return ret;
heap_free( ret );
return NULL;
@@ -2526,42 +2526,42 @@ static WCHAR *get_redirect_url( request_t *request, DWORD *len )
static BOOL handle_redirect( request_t *request, DWORD status )
{
BOOL ret = FALSE;
- DWORD len, len_url;
+ DWORD len, len_loc;
URL_COMPONENTS uc;
connect_t *connect = request->connect;
INTERNET_PORT port;
WCHAR *hostname = NULL, *location;
int index;
- if (!(location = get_redirect_url( request, &len_url ))) return FALSE;
+ if (!(location = get_redirect_url( request, &len_loc ))) return FALSE;
memset( &uc, 0, sizeof(uc) );
uc.dwStructSize = sizeof(uc);
uc.dwSchemeLength = uc.dwHostNameLength = uc.dwUrlPathLength = uc.dwExtraInfoLength = ~0u;
- if (!WinHttpCrackUrl( location, len_url, 0, &uc )) /* assume relative redirect */
+ if (!WinHttpCrackUrl( location, len_loc, 0, &uc )) /* assume relative redirect */
{
WCHAR *path, *p;
if (location[0] == '/')
{
- len = strlenW( location );
+ len = escape_string( NULL, location, len_loc );
if (!(path = heap_alloc( (len + 1) * sizeof(WCHAR) ))) goto end;
- strcpyW( path, location );
+ escape_string( path, location, len_loc );
}
else
{
if ((p = strrchrW( request->path, '/' ))) *p = 0;
- len = strlenW( request->path ) + 1 + strlenW( location );
+ len = strlenW( request->path ) + 1 + escape_string( NULL, location, len_loc );
if (!(path = heap_alloc( (len + 1) * sizeof(WCHAR) ))) goto end;
strcpyW( path, request->path );
strcatW( path, slashW );
- strcatW( path, location );
+ escape_string( path + strlenW(path), location, len_loc );
}
heap_free( request->path );
request->path = path;
- send_callback( &request->hdr, WINHTTP_CALLBACK_STATUS_REDIRECT, location, len_url + 1 );
+ send_callback( &request->hdr, WINHTTP_CALLBACK_STATUS_REDIRECT, location, len_loc + 1 );
}
else
{
@@ -2577,7 +2577,7 @@ static BOOL handle_redirect( request_t *request, DWORD status )
request->hdr.flags |= WINHTTP_FLAG_SECURE;
}
- send_callback( &request->hdr, WINHTTP_CALLBACK_STATUS_REDIRECT, location, len_url + 1 );
+ send_callback( &request->hdr, WINHTTP_CALLBACK_STATUS_REDIRECT, location, len_loc + 1 );
len = uc.dwHostNameLength;
if (!(hostname = heap_alloc( (len + 1) * sizeof(WCHAR) ))) goto end;
@@ -2607,9 +2607,9 @@ static BOOL handle_redirect( request_t *request, DWORD status )
request->path = NULL;
if (uc.dwUrlPathLength)
{
- len = uc.dwUrlPathLength + uc.dwExtraInfoLength;
+ len = escape_string( NULL, uc.lpszUrlPath, uc.dwUrlPathLength + uc.dwExtraInfoLength );
if (!(request->path = heap_alloc( (len + 1) * sizeof(WCHAR) ))) goto end;
- strcpyW( request->path, uc.lpszUrlPath );
+ escape_string( request->path, uc.lpszUrlPath, uc.dwUrlPathLength + uc.dwExtraInfoLength );
}
else request->path = strdupW( slashW );
}
diff --git a/dlls/winhttp/session.c b/dlls/winhttp/session.c
index 788f7c613f..8da09a2e21 100644
--- a/dlls/winhttp/session.c
+++ b/dlls/winhttp/session.c
@@ -1142,14 +1142,14 @@ HINTERNET WINAPI WinHttpOpenRequest( HINTERNET hconnect, LPCWSTR verb, LPCWSTR o
if (object)
{
WCHAR *path, *p;
- unsigned int len;
+ unsigned int len, len_object = strlenW(object);
- len = strlenW( object ) + 1;
+ len = escape_string( NULL, object, len_object );
if (object[0] != '/') len++;
- if (!(p = path = heap_alloc( len * sizeof(WCHAR) ))) goto end;
+ if (!(p = path = heap_alloc( (len + 1) * sizeof(WCHAR) ))) goto end;
if (object[0] != '/') *p++ = '/';
- strcpyW( p, object );
+ escape_string( p, object, len_object );
request->path = path;
}
else if (!(request->path = strdupW( slashW ))) goto end;
diff --git a/dlls/winhttp/tests/url.c b/dlls/winhttp/tests/url.c
index 6bde1de70e..214b7f34cf 100644
--- a/dlls/winhttp/tests/url.c
+++ b/dlls/winhttp/tests/url.c
@@ -667,6 +667,7 @@ static void WinHttpCrackUrl_test( void )
ok( ret, "WinHttpCrackUrl failed le=%u\n", GetLastError() );
ok( !lstrcmpW( uc.lpszHostName, hostnameW ), "unexpected host name\n" );
ok( !lstrcmpW( uc.lpszUrlPath, pathW ), "unexpected path\n" );
+ ok( uc.dwUrlPathLength == lstrlenW(pathW), "got %u\n", uc.dwUrlPathLength );
uc.dwStructSize = sizeof(uc);
uc.lpszScheme = NULL;
diff --git a/dlls/winhttp/tests/winhttp.c b/dlls/winhttp/tests/winhttp.c
index 2e903e1105..df822b2e46 100644
--- a/dlls/winhttp/tests/winhttp.c
+++ b/dlls/winhttp/tests/winhttp.c
@@ -2274,6 +2274,11 @@ static DWORD CALLBACK server_thread(LPVOID param)
if (!strstr(buffer, "Cookie: name=value\r\n")) send(c, cookiemsg, sizeof(cookiemsg) - 1, 0);
else send(c, notokmsg, sizeof(notokmsg) - 1, 0);
}
+ else if (strstr(buffer, "GET /escape"))
+ {
+ if (strstr(buffer, "GET /escape?one%20two%0D%0A HTTP/1.1")) send(c, okmsg, sizeof(okmsg) - 1, 0);
+ else send(c, notokmsg, sizeof(notokmsg) - 1, 0);
+ }
if (strstr(buffer, "GET /quit"))
{
send(c, okmsg, sizeof okmsg - 1, 0);
@@ -3229,6 +3234,40 @@ static void test_cookies( int port )
WinHttpCloseHandle( ses );
}
+static void test_request_path_escapes( int port )
+{
+ static const WCHAR objW[] =
+ {'/','e','s','c','a','p','e','?','o','n','e',' ','t','w','o','\r','\n',0};
+ HINTERNET ses, con, req;
+ DWORD status, size;
+ BOOL ret;
+
+ ses = WinHttpOpen( test_useragent, WINHTTP_ACCESS_TYPE_NO_PROXY, NULL, NULL, 0 );
+ ok( ses != NULL, "failed to open session %u\n", GetLastError() );
+
+ con = WinHttpConnect( ses, localhostW, port, 0 );
+ ok( con != NULL, "failed to open a connection %u\n", GetLastError() );
+
+ req = WinHttpOpenRequest( con, NULL, objW, NULL, NULL, NULL, 0 );
+ ok( req != NULL, "failed to open a request %u\n", GetLastError() );
+
+ ret = WinHttpSendRequest( req, NULL, 0, NULL, 0, 0, 0 );
+ ok( ret, "failed to send request %u\n", GetLastError() );
+
+ ret = WinHttpReceiveResponse( req, NULL );
+ ok( ret, "failed to receive response %u\n", GetLastError() );
+
+ status = 0xdeadbeef;
+ size = sizeof(status);
+ ret = WinHttpQueryHeaders( req, WINHTTP_QUERY_STATUS_CODE|WINHTTP_QUERY_FLAG_NUMBER, NULL, &status, &size, NULL );
+ ok( ret, "failed to query status code %u\n", GetLastError() );
+ ok( status == HTTP_STATUS_OK, "request failed unexpectedly %u\n", status );
+
+ WinHttpCloseHandle( req );
+ WinHttpCloseHandle( con );
+ WinHttpCloseHandle( ses );
+}
+
static void test_connection_info( int port )
{
static const WCHAR basicW[] = {'/','b','a','s','i','c',0};
@@ -4555,6 +4594,7 @@ START_TEST (winhttp)
test_bad_header(si.port);
test_multiple_reads(si.port);
test_cookies(si.port);
+ test_request_path_escapes(si.port);
/* send the basic request again to shutdown the server thread */
test_basic_request(si.port, NULL, quitW);
diff --git a/dlls/winhttp/url.c b/dlls/winhttp/url.c
index 32b30da0ce..52a175c84d 100644
--- a/dlls/winhttp/url.c
+++ b/dlls/winhttp/url.c
@@ -118,7 +118,7 @@ static BOOL need_escape( WCHAR c )
}
}
-static DWORD copy_escape( WCHAR *dst, const WCHAR *src, DWORD len )
+DWORD escape_string( WCHAR *dst, const WCHAR *src, DWORD len )
{
static const WCHAR hex[] = {'0','1','2','3','4','5','6','7','8','9','A','B','C','D','E','F'};
DWORD ret = len;
@@ -129,38 +129,45 @@ static DWORD copy_escape( WCHAR *dst, const WCHAR *src, DWORD len )
{
if (need_escape( src[i] ))
{
- p[0] = '%';
- p[1] = hex[(src[i] >> 4) & 0xf];
- p[2] = hex[src[i] & 0xf];
+ if (dst)
+ {
+ p[0] = '%';
+ p[1] = hex[(src[i] >> 4) & 0xf];
+ p[2] = hex[src[i] & 0xf];
+ p += 2;
+ }
ret += 2;
- p += 2;
}
- else *p = src[i];
+ else if (dst) *p = src[i];
}
- dst[ret] = 0;
+ if (dst) dst[ret] = 0;
return ret;
}
-static WCHAR *escape_url( LPCWSTR url, DWORD *len )
+static WCHAR *escape_url( const WCHAR *url, DWORD *len )
{
WCHAR *ret;
- const WCHAR *p, *q;
+ const WCHAR *p;
+ DWORD len_base, len_path;
- if ((p = q = strrchrW( url, '/' )))
+ if ((p = strrchrW( url, '/' )))
{
- while (*q)
- {
- if (need_escape( *q )) *len += 2;
- q++;
- }
+ len_base = p - url;
+ len_path = escape_string( NULL, p, *len - len_base );
}
- if (!(ret = heap_alloc( (*len + 1) * sizeof(WCHAR) ))) return NULL;
- if (!p) strcpyW( ret, url );
else
{
- memcpy( ret, url, (p - url) * sizeof(WCHAR) );
- copy_escape( ret + (p - url), p, q - p );
+ len_base = *len;
+ len_path = 0;
}
+
+ if (!(ret = heap_alloc( (len_base + len_path + 1) * sizeof(WCHAR) ))) return NULL;
+ memcpy( ret, url, len_base * sizeof(WCHAR) );
+
+ if (p) escape_string( ret + len_base, p, *len - (p - url) );
+ ret[len_base + len_path] = 0;
+
+ *len = len_base + len_path;
return ret;
}
@@ -516,7 +523,7 @@ BOOL WINAPI WinHttpCreateUrl( LPURL_COMPONENTS uc, DWORD flags, LPWSTR url, LPDW
if (uc->lpszUrlPath)
{
len = comp_length( uc->dwUrlPathLength, 0, uc->lpszUrlPath );
- if (flags & ICU_ESCAPE) url += copy_escape( url, uc->lpszUrlPath, len );
+ if (flags & ICU_ESCAPE) url += escape_string( url, uc->lpszUrlPath, len );
else
{
memcpy( url, uc->lpszUrlPath, len * sizeof(WCHAR) );
@@ -526,7 +533,7 @@ BOOL WINAPI WinHttpCreateUrl( LPURL_COMPONENTS uc, DWORD flags, LPWSTR url, LPDW
if (uc->lpszExtraInfo)
{
len = comp_length( uc->dwExtraInfoLength, 0, uc->lpszExtraInfo );
- if (flags & ICU_ESCAPE) url += copy_escape( url, uc->lpszExtraInfo, len );
+ if (flags & ICU_ESCAPE) url += escape_string( url, uc->lpszExtraInfo, len );
else
{
memcpy( url, uc->lpszExtraInfo, len * sizeof(WCHAR) );
diff --git a/dlls/winhttp/winhttp_private.h b/dlls/winhttp/winhttp_private.h
index f1aa5622c6..7b92afc7ce 100644
--- a/dlls/winhttp/winhttp_private.h
+++ b/dlls/winhttp/winhttp_private.h
@@ -322,6 +322,7 @@ BOOL set_server_for_hostname( connect_t *, LPCWSTR, INTERNET_PORT ) DECLSPEC_HID
void destroy_authinfo( struct authinfo * ) DECLSPEC_HIDDEN;
void release_host( hostdata_t *host ) DECLSPEC_HIDDEN;
+DWORD escape_string( WCHAR *, const WCHAR *, DWORD ) DECLSPEC_HIDDEN;
extern HRESULT WinHttpRequest_create( void ** ) DECLSPEC_HIDDEN;
void release_typelib( void ) DECLSPEC_HIDDEN;
--
2.11.0
More information about the wine-devel
mailing list