[1/2] winhttp: Set required buffer length for all components in WinHttpCrackUrl.

Hans Leidekker hans at codeweavers.com
Tue Aug 16 05:30:01 CDT 2016


Signed-off-by: Hans Leidekker <hans at codeweavers.com>
---
 dlls/winhttp/tests/url.c |  49 ++++++++++++-----
 dlls/winhttp/url.c       | 139 ++++++++++++++++++++++++++++++-----------------
 2 files changed, 124 insertions(+), 64 deletions(-)

diff --git a/dlls/winhttp/tests/url.c b/dlls/winhttp/tests/url.c
index 7935a19..ec6dbd0 100644
--- a/dlls/winhttp/tests/url.c
+++ b/dlls/winhttp/tests/url.c
@@ -330,12 +330,7 @@ static void WinHttpCrackUrl_test( void )
     BOOL ret;
 
     /* buffers of sufficient length */
-    scheme[0] = 0;
-    user[0] = 0;
-    pass[0] = 0;
-    host[0] = 0;
-    path[0] = 0;
-    extra[0] = 0;
+    scheme[0] = user[0] = pass[0] = host[0] = path[0] = extra[0] = 0;
 
     uc.dwStructSize = sizeof(URL_COMPONENTS);
     uc.nScheme = 0;
@@ -370,23 +365,24 @@ static void WinHttpCrackUrl_test( void )
     ok( !memcmp( uc.lpszExtraInfo, query, sizeof(query) ), "unexpected extra info: %s\n", wine_dbgstr_w(uc.lpszExtraInfo) );
     ok( uc.dwExtraInfoLength == 6, "unexpected extra info length: %u\n", uc.dwExtraInfoLength );
 
-    /* buffer of insufficient length */
-    scheme[0] = 0;
-    uc.dwSchemeLength = 1;
-
+    /* buffers of insufficient length */
+    uc.dwSchemeLength   = 1;
+    uc.dwHostNameLength = 1;
+    uc.dwUrlPathLength  = 40; /* sufficient */
     SetLastError( 0xdeadbeef );
     ret = WinHttpCrackUrl( url1, 0, 0, &uc );
     error = GetLastError();
     ok( !ret, "WinHttpCrackUrl succeeded\n" );
     ok( error == ERROR_INSUFFICIENT_BUFFER, "got %u, expected ERROR_INSUFFICIENT_BUFFER\n", error );
     ok( uc.dwSchemeLength == 5, "unexpected scheme length: %u\n", uc.dwSchemeLength );
+    ok( uc.dwHostNameLength == 15, "unexpected hostname length: %u\n", uc.dwHostNameLength );
+    ok( uc.dwUrlPathLength == 11, "unexpected path length: %u\n", uc.dwUrlPathLength );
 
     /* no buffers */
     reset_url_components( &uc );
     SetLastError( 0xdeadbeef );
-    ret = WinHttpCrackUrl( url_k1, 0, 0,&uc);
+    ret = WinHttpCrackUrl( url_k1, 0, 0, &uc);
     error = GetLastError();
-
     ok( ret, "WinHttpCrackUrl failed le=%u\n", error );
     ok( error == ERROR_SUCCESS || broken(error == ERROR_INVALID_PARAMETER) /* < win7 */,
         "got %u, expected ERROR_SUCCESS\n", error );
@@ -407,7 +403,6 @@ static void WinHttpCrackUrl_test( void )
 
     reset_url_components( &uc );
     ret = WinHttpCrackUrl( url_k2, 0, 0,&uc);
-
     ok( ret, "WinHttpCrackUrl failed le=%u\n", GetLastError() );
     ok( uc.nScheme == INTERNET_SCHEME_HTTP, "unexpected scheme\n" );
     ok( uc.lpszScheme == url_k2, "unexpected scheme\n" );
@@ -426,7 +421,6 @@ static void WinHttpCrackUrl_test( void )
 
     reset_url_components( &uc );
     ret = WinHttpCrackUrl( url_k3, 0, 0, &uc );
-
     ok( ret, "WinHttpCrackUrl failed le=%u\n", GetLastError() );
     ok( uc.nScheme == INTERNET_SCHEME_HTTPS, "unexpected scheme\n" );
     ok( uc.lpszScheme == url_k3, "unexpected scheme\n" );
@@ -445,24 +439,39 @@ static void WinHttpCrackUrl_test( void )
 
     /* bad parameters */
     reset_url_components( &uc );
+    SetLastError( 0xdeadbeef );
     ret = WinHttpCrackUrl( url_k4, 0, 0, &uc );
     ok( !ret, "WinHttpCrackUrl succeeded\n" );
+    error = GetLastError();
+    ok( error == ERROR_WINHTTP_INVALID_URL, "got %u\n", error );
 
     reset_url_components( &uc );
+    SetLastError( 0xdeadbeef );
     ret = WinHttpCrackUrl( url_k5, 0, 0, &uc );
     ok( !ret, "WinHttpCrackUrl succeeded\n" );
+    error = GetLastError();
+    ok( error == ERROR_WINHTTP_INVALID_URL, "got %u\n", error );
 
     reset_url_components( &uc );
+    SetLastError( 0xdeadbeef );
     ret = WinHttpCrackUrl( url_k6, 0, 0, &uc );
     ok( !ret, "WinHttpCrackUrl succeeded\n" );
+    error = GetLastError();
+    ok( error == ERROR_WINHTTP_UNRECOGNIZED_SCHEME, "got %u\n", error );
 
     reset_url_components( &uc );
+    SetLastError( 0xdeadbeef );
     ret = WinHttpCrackUrl( url_k7, 0, 0, &uc );
     ok( !ret, "WinHttpCrackUrl succeeded\n" );
+    error = GetLastError();
+    ok( error == ERROR_WINHTTP_UNRECOGNIZED_SCHEME, "got %u\n", error );
 
     reset_url_components( &uc );
+    SetLastError( 0xdeadbeef );
     ret = WinHttpCrackUrl( url_k8, 0, 0, &uc );
+    error = GetLastError();
     ok( !ret, "WinHttpCrackUrl succeeded\n" );
+    ok( error == ERROR_WINHTTP_UNRECOGNIZED_SCHEME, "got %u\n", error );
 
     reset_url_components( &uc );
     ret = WinHttpCrackUrl( url_k9, 0, 0, &uc );
@@ -484,18 +493,30 @@ static void WinHttpCrackUrl_test( void )
     ok( uc.dwExtraInfoLength == 0, "unexpected extra info length: %u\n", uc.dwExtraInfoLength );
 
     reset_url_components( &uc );
+    SetLastError( 0xdeadbeef );
     ret = WinHttpCrackUrl( url4, 0, 0, &uc );
+    error = GetLastError();
     ok( !ret, "WinHttpCrackUrl succeeded\n" );
+    ok( error == ERROR_WINHTTP_INVALID_URL, "got %u\n", error );
 
     reset_url_components( &uc );
+    SetLastError( 0xdeadbeef );
     ret = WinHttpCrackUrl( empty, 0, 0, &uc );
+    error = GetLastError();
     ok( !ret, "WinHttpCrackUrl succeeded\n" );
+    ok( error == ERROR_WINHTTP_UNRECOGNIZED_SCHEME, "got %u\n", error );
 
+    SetLastError( 0xdeadbeef );
     ret = WinHttpCrackUrl( url1, 0, 0, NULL );
+    error = GetLastError();
     ok( !ret, "WinHttpCrackUrl succeeded\n" );
+    ok( error == ERROR_INVALID_PARAMETER, "got %u\n", error );
 
+    SetLastError( 0xdeadbeef );
     ret = WinHttpCrackUrl( NULL, 0, 0, &uc );
+    error = GetLastError();
     ok( !ret, "WinHttpCrackUrl succeeded\n" );
+    ok( error == ERROR_INVALID_PARAMETER, "got %u\n", error );
 
     /* decoding without buffers */
     reset_url_components( &uc );
diff --git a/dlls/winhttp/url.c b/dlls/winhttp/url.c
index 8469ea3..2b479a9 100644
--- a/dlls/winhttp/url.c
+++ b/dlls/winhttp/url.c
@@ -34,37 +34,28 @@ WINE_DEFAULT_DEBUG_CHANNEL(winhttp);
 static const WCHAR scheme_http[] = {'h','t','t','p',0};
 static const WCHAR scheme_https[] = {'h','t','t','p','s',0};
 
-static BOOL set_component( WCHAR **str, DWORD *str_len, WCHAR *value, DWORD len, DWORD flags )
+static DWORD set_component( WCHAR **str, DWORD *str_len, WCHAR *value, DWORD len, DWORD flags )
 {
-    if (*str && !*str_len)
-    {
-        set_last_error( ERROR_INVALID_PARAMETER );
-        return FALSE;
-    }
-    if (!*str_len) return TRUE;
+    if (*str && !*str_len) return ERROR_INVALID_PARAMETER;
+    if (!*str_len) return ERROR_SUCCESS;
     if (!*str)
     {
-        if (len && *str_len && (flags & (ICU_DECODE|ICU_ESCAPE)))
-        {
-            set_last_error( ERROR_INVALID_PARAMETER );
-            return FALSE;
-        }
+        if (len && *str_len && (flags & (ICU_DECODE|ICU_ESCAPE))) return ERROR_INVALID_PARAMETER;
         *str = value;
         *str_len = len;
     }
     else
     {
-        if (len > (*str_len) - 1)
+        if (len >= *str_len)
         {
-            *str_len = len + 1;
-            set_last_error( ERROR_INSUFFICIENT_BUFFER );
-            return FALSE;
+            *str_len = len;
+            return ERROR_SUCCESS;
         }
         memcpy( *str, value, len * sizeof(WCHAR) );
         (*str)[len] = 0;
         *str_len = len;
     }
-    return TRUE;
+    return ERROR_SUCCESS;
 }
 
 static WCHAR *decode_url( LPCWSTR url, DWORD *len )
@@ -177,8 +168,8 @@ static WCHAR *escape_url( LPCWSTR url, DWORD *len )
  */
 BOOL WINAPI WinHttpCrackUrl( LPCWSTR url, DWORD len, DWORD flags, LPURL_COMPONENTSW uc )
 {
-    BOOL ret = FALSE;
     WCHAR *p, *q, *r, *url_decoded = NULL, *url_escaped = NULL;
+    DWORD err, scheme_len, user_len, passwd_len, host_len, path_len, extra_len;
     INTERNET_SCHEME scheme = 0;
 
     TRACE("%s, %d, %x, %p\n", debugstr_w(url), len, flags, uc);
@@ -189,6 +180,12 @@ BOOL WINAPI WinHttpCrackUrl( LPCWSTR url, DWORD len, DWORD flags, LPURL_COMPONEN
         return FALSE;
     }
     if (!len) len = strlenW( url );
+    scheme_len = uc->dwSchemeLength;
+    host_len   = uc->dwHostNameLength;
+    user_len   = uc->dwUserNameLength;
+    passwd_len = uc->dwPasswordLength;
+    path_len   = uc->dwUrlPathLength;
+    extra_len  = uc->dwExtraInfoLength;
 
     if (flags & ICU_ESCAPE)
     {
@@ -217,95 +214,137 @@ BOOL WINAPI WinHttpCrackUrl( LPCWSTR url, DWORD len, DWORD flags, LPURL_COMPONEN
     else if (p - url == 5 && !strncmpiW( url, scheme_https, 5 )) scheme = INTERNET_SCHEME_HTTPS;
     else
     {
-        set_last_error( ERROR_WINHTTP_UNRECOGNIZED_SCHEME );
+        err = ERROR_WINHTTP_UNRECOGNIZED_SCHEME;
         goto exit;
     }
-    if (!(set_component( &uc->lpszScheme, &uc->dwSchemeLength, (WCHAR *)url, p - url, flags ))) goto exit;
+
+    if ((err = set_component( &uc->lpszScheme, &scheme_len, (WCHAR *)url, p - url, flags ))) goto exit;
 
     p++; /* skip ':' */
-    if (!p[0] || p[0] != '/' || p[1] != '/') goto exit;
+    if (!p[0] || p[0] != '/' || p[1] != '/')
+    {
+        err = ERROR_WINHTTP_INVALID_URL;
+        goto exit;
+    }
     p += 2;
-
-    if (!p[0]) goto exit;
+    if (!p[0])
+    {
+        err = ERROR_WINHTTP_INVALID_URL;
+        goto exit;
+    }
     if ((q = memchrW( p, '@', len - (p - url) )) && !(memchrW( p, '/', q - p )))
     {
         if ((r = memchrW( p, ':', q - p )))
         {
-            if (!(set_component( &uc->lpszUserName, &uc->dwUserNameLength, p, r - p, flags ))) goto exit;
+            if ((err = set_component( &uc->lpszUserName, &user_len, p, r - p, flags ))) goto exit;
             r++;
-            if (!(set_component( &uc->lpszPassword, &uc->dwPasswordLength, r, q - r, flags ))) goto exit;
+            if ((err = set_component( &uc->lpszPassword, &passwd_len, r, q - r, flags ))) goto exit;
         }
         else
         {
-            if (!(set_component( &uc->lpszUserName, &uc->dwUserNameLength, p, q - p, flags ))) goto exit;
-            if (!(set_component( &uc->lpszPassword, &uc->dwPasswordLength, NULL, 0, flags ))) goto exit;
+            if ((err = set_component( &uc->lpszUserName, &user_len, p, q - p, flags ))) goto exit;
+            if ((err = set_component( &uc->lpszPassword, &passwd_len, NULL, 0, flags ))) goto exit;
         }
         p = q + 1;
     }
     else
     {
-        if (!(set_component( &uc->lpszUserName, &uc->dwUserNameLength, NULL, 0, flags ))) goto exit;
-        if (!(set_component( &uc->lpszPassword, &uc->dwPasswordLength, NULL, 0, flags ))) goto exit;
+        if ((err = set_component( &uc->lpszUserName, &user_len, NULL, 0, flags ))) goto exit;
+        if ((err = set_component( &uc->lpszPassword, &passwd_len, NULL, 0, flags ))) goto exit;
     }
     if ((q = memchrW( p, '/', len - (p - url) )))
     {
         if ((r = memchrW( p, ':', q - p )))
         {
-            if (!(set_component( &uc->lpszHostName, &uc->dwHostNameLength, p, r - p, flags ))) goto exit;
+            if ((err = set_component( &uc->lpszHostName, &host_len, p, r - p, flags ))) goto exit;
             r++;
             uc->nPort = atoiW( r );
         }
         else
         {
-            if (!(set_component( &uc->lpszHostName, &uc->dwHostNameLength, p, q - p, flags ))) goto exit;
+            if ((err = set_component( &uc->lpszHostName, &host_len, p, q - p, flags ))) goto exit;
             if (scheme == INTERNET_SCHEME_HTTP) uc->nPort = INTERNET_DEFAULT_HTTP_PORT;
             if (scheme == INTERNET_SCHEME_HTTPS) uc->nPort = INTERNET_DEFAULT_HTTPS_PORT;
         }
 
         if ((r = memchrW( q, '?', len - (q - url) )))
         {
-            if (!(set_component( &uc->lpszUrlPath, &uc->dwUrlPathLength, q, r - q, flags ))) goto exit;
-            if (!(set_component( &uc->lpszExtraInfo, &uc->dwExtraInfoLength, r, len - (r - url), flags ))) goto exit;
+            if ((err = set_component( &uc->lpszUrlPath, &path_len, q, r - q, flags ))) goto exit;
+            if ((err = set_component( &uc->lpszExtraInfo, &extra_len, r, len - (r - url), flags ))) goto exit;
         }
         else
         {
-            if (!(set_component( &uc->lpszUrlPath, &uc->dwUrlPathLength, q, len - (q - url), flags ))) goto exit;
-            if (!(set_component( &uc->lpszExtraInfo, &uc->dwExtraInfoLength, (WCHAR *)url + len, 0, flags ))) goto exit;
+            if ((err = set_component( &uc->lpszUrlPath, &path_len, q, len - (q - url), flags ))) goto exit;
+            if ((err = set_component( &uc->lpszExtraInfo, &extra_len, (WCHAR *)url + len, 0, flags ))) goto exit;
         }
     }
     else
     {
         if ((r = memchrW( p, ':', len - (p - url) )))
         {
-            if (!(set_component( &uc->lpszHostName, &uc->dwHostNameLength, p, r - p, flags ))) goto exit;
+            if ((err = set_component( &uc->lpszHostName, &host_len, p, r - p, flags ))) goto exit;
             r++;
             uc->nPort = atoiW( r );
         }
         else
         {
-            if (!(set_component( &uc->lpszHostName, &uc->dwHostNameLength, p, len - (p - url), flags ))) goto exit;
+            if ((err = set_component( &uc->lpszHostName, &host_len, p, len - (p - url), flags ))) goto exit;
             if (scheme == INTERNET_SCHEME_HTTP) uc->nPort = INTERNET_DEFAULT_HTTP_PORT;
             if (scheme == INTERNET_SCHEME_HTTPS) uc->nPort = INTERNET_DEFAULT_HTTPS_PORT;
         }
-        if (!(set_component( &uc->lpszUrlPath, &uc->dwUrlPathLength, (WCHAR *)url + len, 0, flags ))) goto exit;
-        if (!(set_component( &uc->lpszExtraInfo, &uc->dwExtraInfoLength, (WCHAR *)url + len, 0, flags ))) goto exit;
+        if ((err = set_component( &uc->lpszUrlPath, &path_len, (WCHAR *)url + len, 0, flags ))) goto exit;
+        if ((err = set_component( &uc->lpszExtraInfo, &extra_len, (WCHAR *)url + len, 0, flags ))) goto exit;
     }
 
-    ret = TRUE;
-
-    TRACE("scheme(%s) host(%s) port(%d) path(%s) extra(%s)\n",
-          debugstr_wn( uc->lpszScheme, uc->dwSchemeLength ),
-          debugstr_wn( uc->lpszHostName, uc->dwHostNameLength ),
-          uc->nPort,
-          debugstr_wn( uc->lpszUrlPath, uc->dwUrlPathLength ),
-          debugstr_wn( uc->lpszExtraInfo, uc->dwExtraInfoLength ));
+    TRACE("scheme(%s) host(%s) port(%d) path(%s) extra(%s)\n", debugstr_wn( uc->lpszScheme, scheme_len ),
+          debugstr_wn( uc->lpszHostName, host_len ), uc->nPort, debugstr_wn( uc->lpszUrlPath, path_len ),
+          debugstr_wn( uc->lpszExtraInfo, extra_len ));
 
 exit:
-    if (ret) uc->nScheme = scheme;
+    if (!err)
+    {
+        if (uc->lpszScheme && uc->dwSchemeLength <= scheme_len)
+        {
+            scheme_len++;
+            err = ERROR_INSUFFICIENT_BUFFER;
+        }
+        if (uc->lpszHostName && uc->dwHostNameLength <= host_len)
+        {
+            host_len++;
+            err = ERROR_INSUFFICIENT_BUFFER;
+        }
+        if (uc->lpszUserName && uc->dwUserNameLength <= user_len)
+        {
+            user_len++;
+            err = ERROR_INSUFFICIENT_BUFFER;
+        }
+        if (uc->lpszPassword && uc->dwPasswordLength <= passwd_len)
+        {
+            passwd_len++;
+            err = ERROR_INSUFFICIENT_BUFFER;
+        }
+        if (uc->lpszUrlPath && uc->dwUrlPathLength <= path_len)
+        {
+            path_len++;
+            err = ERROR_INSUFFICIENT_BUFFER;
+        }
+        if (uc->lpszExtraInfo && uc->dwExtraInfoLength <= extra_len)
+        {
+            extra_len++;
+            err = ERROR_INSUFFICIENT_BUFFER;
+        }
+        uc->nScheme           = scheme;
+        uc->dwSchemeLength    = scheme_len;
+        uc->dwHostNameLength  = host_len;
+        uc->dwUserNameLength  = user_len;
+        uc->dwPasswordLength  = passwd_len;
+        uc->dwUrlPathLength   = path_len;
+        uc->dwExtraInfoLength = extra_len;
+    }
     heap_free( url_decoded );
     heap_free( url_escaped );
-    if (ret) set_last_error( ERROR_SUCCESS );
-    return ret;
+    set_last_error( err );
+    return !err;
 }
 
 static INTERNET_SCHEME get_scheme( const WCHAR *scheme, DWORD len )
-- 
2.1.4




More information about the wine-patches mailing list