wininet: fixed the handling of insufficient buffers in InternetGetCookie*().

Eric van Beurden ericvb at transgaming.com
Tue Jun 29 10:23:17 CDT 2010


---
 dlls/wininet/cookie.c         |  181 ++++++++++++++++++++++++++++++++---------
 dlls/wininet/tests/internet.c |  181 +++++++++++++++++++++++++++++++++++++++++
 2 files changed, 324 insertions(+), 38 deletions(-)

diff --git a/dlls/wininet/cookie.c b/dlls/wininet/cookie.c
index 75cf049..09e4161 100644
--- a/dlls/wininet/cookie.c
+++ b/dlls/wininet/cookie.c
@@ -265,22 +265,21 @@ static void COOKIE_deleteDomain(cookie_domain *deadDomain)
  *
  * Retrieve cookie from the specified url
  *
- *  It should be noted that on windows the lpszCookieName parameter is "not implemented".
- *    So it won't be implemented here.
- *
  * RETURNS
  *    TRUE  on success
  *    FALSE on failure
  *
  */
-BOOL WINAPI InternetGetCookieW(LPCWSTR lpszUrl, LPCWSTR lpszCookieName,
-    LPWSTR lpCookieData, LPDWORD lpdwSize)
+static BOOL DoInternetGetCookieW(LPCWSTR lpszUrl, LPCWSTR lpszCookieName,
+    LPWSTR lpCookieData, LPDWORD lpdwSize, BOOL ignoreName)
 {
     BOOL ret;
     struct list * cursor;
     unsigned int cnt = 0, domain_count = 0, cookie_count = 0;
     WCHAR hostName[2048], path[2048];
     FILETIME tm;
+    BOOL overflow = FALSE;
+    BOOL justCount;
 
     TRACE("(%s, %s, %p, %p)\n", debugstr_w(lpszUrl),debugstr_w(lpszCookieName),
           lpCookieData, lpdwSize);
@@ -291,6 +290,12 @@ BOOL WINAPI InternetGetCookieW(LPCWSTR lpszUrl, LPCWSTR lpszCookieName,
         return FALSE;
     }
 
+    justCount = (lpCookieData == NULL);
+
+    /* no point in checking the cookie names if none is given */
+    if (lpszCookieName == NULL)
+        ignoreName = TRUE;
+
     hostName[0] = 0;
     ret = COOKIE_crackUrlSimple(lpszUrl, hostName, sizeof(hostName)/sizeof(hostName[0]), path, sizeof(path)/sizeof(path[0]));
     if (!ret || !hostName[0]) return FALSE;
@@ -309,6 +314,10 @@ BOOL WINAPI InternetGetCookieW(LPCWSTR lpszUrl, LPCWSTR lpszCookieName,
             LIST_FOR_EACH(cursor, &cookiesDomain->cookie_list)
             {
                 cookie *thisCookie = LIST_ENTRY(cursor, cookie, entry);
+                DWORD   len;
+                size_t  cookieNameLen;
+                size_t  cookieValueLen = 0;
+
                 /* check for expiry */
                 if ((thisCookie->expiry.dwLowDateTime != 0 || thisCookie->expiry.dwHighDateTime != 0) && CompareFileTime(&tm,&thisCookie->expiry)  > 0)
                 {
@@ -317,32 +326,61 @@ BOOL WINAPI InternetGetCookieW(LPCWSTR lpszUrl, LPCWSTR lpszCookieName,
                     continue;
                 }
 
-                if (lpCookieData == NULL) /* return the size of the buffer required to lpdwSize */
+                /* this is not the cookie we're looking for => skip to the next one */
+                /* note that cookie names are *not* case sensitive */
+                if (!ignoreName && strcmpiW(thisCookie->lpCookieName, lpszCookieName))
+                    continue;
+
+                /* figure out how many characters this cookie and value will occupy */
+                len = 0;
+                if (cookie_count) len += 2; /* '; ' */
+                cookieNameLen = strlenW(thisCookie->lpCookieName);
+                len += cookieNameLen;
+                if (thisCookie->lpCookieData[0])
                 {
-                    unsigned int len;
+                    len += 1; /* = */
+                    cookieValueLen = strlenW(thisCookie->lpCookieData);
+                    len += cookieValueLen;
+                }
+
+                /* attempt to write to the buffer.  Testing shows that the native version does not attempt to
+                   write partial cookie name/value pairs to the buffer if it runs out of space. */
+                if (!justCount && cnt + len < *lpdwSize)
+                {
+                    static const WCHAR szsc[] = {';', ' ', 0};
+
 
-                    if (cookie_count) cnt += 2; /* '; ' */
-                    cnt += strlenW(thisCookie->lpCookieName);
-                    if ((len = strlenW(thisCookie->lpCookieData)))
+                    if (cookie_count)
                     {
-                        cnt += 1; /* = */
-                        cnt += len;
+                        lstrcpyW(lpCookieData + cnt, szsc);
+                        cnt += strlenW(szsc);
                     }
-                }
-                else
-                {
-                    static const WCHAR szsc[] = { ';',' ',0 };
-                    static const WCHAR szname[] = { '%','s',0 };
-                    static const WCHAR szdata[] = { '=','%','s',0 };
 
-                    if (cookie_count) cnt += snprintfW(lpCookieData + cnt, *lpdwSize - cnt, szsc);
-                    cnt += snprintfW(lpCookieData + cnt, *lpdwSize - cnt, szname, thisCookie->lpCookieName);
+                    lstrcpyW(lpCookieData + cnt, thisCookie->lpCookieName);
+                    cnt += cookieNameLen;
 
                     if (thisCookie->lpCookieData[0])
-                        cnt += snprintfW(lpCookieData + cnt, *lpdwSize - cnt, szdata, thisCookie->lpCookieData);
+                    {
+                        lpCookieData[cnt++] = '=';
+                        lstrcpyW(lpCookieData + cnt, thisCookie->lpCookieData);
+                        cnt += cookieValueLen;
+                    }
 
                     TRACE("Cookie: %s\n", debugstr_w(lpCookieData));
                 }
+
+                else
+                {
+                    /* the buffer would have overflowed when adding this cookie => mark the error and continue counting */
+                    if (!justCount)
+                    {
+                        overflow = TRUE;
+                        justCount = TRUE;
+                    }
+
+                    cnt += len;
+                }
+
                 cookie_count++;
             }
         }
@@ -355,6 +393,15 @@ BOOL WINAPI InternetGetCookieW(LPCWSTR lpszUrl, LPCWSTR lpszCookieName,
         return FALSE;
     }
 
+    /* insufficent buffer to store the result => return the required amount */
+    if (overflow)
+    {
+        TRACE("ran out of buffer space.  Given %u, required %d\n", *lpdwSize, cnt + 1);
+        *lpdwSize = (cnt + 1) * sizeof(WCHAR);
+        SetLastError(ERROR_INSUFFICIENT_BUFFER);
+        return FALSE;
+    }
+
     if (lpCookieData == NULL)
     {
         *lpdwSize = (cnt + 1) * sizeof(WCHAR);
@@ -364,12 +411,19 @@ BOOL WINAPI InternetGetCookieW(LPCWSTR lpszUrl, LPCWSTR lpszCookieName,
 
     *lpdwSize = cnt + 1;
 
-    TRACE("Returning %u (from %u domains): %s\n", cnt, domain_count,
-           debugstr_w(lpCookieData));
+    TRACE("Returning %u (from %u domains): %s {cookie_count = %d}\n", cnt, domain_count,
+           debugstr_w(lpCookieData), cookie_count);
 
     return (cnt ? TRUE : FALSE);
 }
 
+BOOL WINAPI InternetGetCookieW(LPCWSTR lpszUrl, LPCWSTR lpszCookieName,
+    LPWSTR lpCookieData, LPDWORD lpdwSize)
+{
+    /* testing shows that the wide version of this function does actually respect the provided
+       cookie name and will only return information about that specific cookie. */
+    return DoInternetGetCookieW(lpszUrl, lpszCookieName, lpCookieData, lpdwSize, FALSE);
+}
 
 /***********************************************************************
  *           InternetGetCookieA (WININET.@)
@@ -385,35 +439,86 @@ BOOL WINAPI InternetGetCookieA(LPCSTR lpszUrl, LPCSTR lpszCookieName,
     LPSTR lpCookieData, LPDWORD lpdwSize)
 {
     DWORD len;
-    LPWSTR szCookieData = NULL, url, name;
-    BOOL r;
+    LPWSTR szCookieData = NULL, url = NULL, name = NULL;
+    BOOL r = FALSE;
+    WCHAR dummy;
 
     TRACE("(%s,%s,%p)\n", debugstr_a(lpszUrl), debugstr_a(lpszCookieName),
         lpCookieData);
 
+    if (lpdwSize == NULL)
+    {
+        SetLastError(ERROR_INVALID_PARAMETER);
+        return FALSE;
+    }
+
     url = heap_strdupAtoW(lpszUrl);
     name = heap_strdupAtoW(lpszCookieName);
 
-    r = InternetGetCookieW( url, name, NULL, &len );
-    if( r )
+    if ((url == NULL && lpszUrl != NULL) ||
+        (name == NULL && lpszCookieName != NULL))
     {
-        szCookieData = HeapAlloc( GetProcessHeap(), 0, len * sizeof(WCHAR) );
-        if( !szCookieData )
+        ERR("not enough memory for the URL or cookie name\n");
+        SetLastError(ERROR_NOT_ENOUGH_MEMORY);
+        goto Done;
+    }
+    if (lpCookieData)
+    {
+        /* a non-zero length buffer was provided => allocate a buffer for the wide version */
+        if (*lpdwSize)
         {
-            r = FALSE;
+            szCookieData = HeapAlloc(GetProcessHeap(), 0, (*lpdwSize) * sizeof(WCHAR));
+
+            if (szCookieData == NULL)
+            {
+                ERR("not enough memory for the output buffer\n");
+                SetLastError(ERROR_NOT_ENOUGH_MEMORY);
+                goto Done;
+            }
         }
+
+        /* a buffer was provided but its size was reported as 0 => use a dummy buffer so that we
+             aren't passing NULL to the function. */
         else
-        {
-            r = InternetGetCookieW( url, name, szCookieData, &len );
+            szCookieData = &dummy;
+    }
 
-            *lpdwSize = WideCharToMultiByte( CP_ACP, 0, szCookieData, len,
-                                    lpCookieData, *lpdwSize, NULL, NULL );
-        }
+    /* save the original input buffer length */
+    len = *lpdwSize;
+
+    /* testing shows that the ANSI version of the function *always* returns the full cookie list
+       even if a valid cookie name is specified. */
+    r = DoInternetGetCookieW( url, name, szCookieData, lpdwSize, TRUE);
+
+    /* function succeeded and possibly returned a string */
+    if (r)
+    {
+        /* no output buffer provided => just correct the required length.  When no buffer is given,
+           the wide version of the function returns the size in bytes, not characters. */
+        if (lpCookieData == NULL)
+            *lpdwSize /= sizeof(WCHAR);
+
+        /* an output buffer was provided => convert the output string.  Note that on success, the
+           <lpdwSize> parameter already returns the resulting string size in characters, so there
+           is no need to convert it. */
+        else
+            WideCharToMultiByte(CP_ACP, 0, szCookieData, *lpdwSize, lpCookieData, len, NULL, NULL);
     }
 
-    HeapFree( GetProcessHeap(), 0, szCookieData );
-    HeapFree( GetProcessHeap(), 0, name );
-    HeapFree( GetProcessHeap(), 0, url );
+    /* function failed due to a buffer overflow => correct the required length.  On this failure,
+       the <lpdwSize> parameter is returned as the number of bytes, not characters. */
+    else if (GetLastError() == ERROR_INSUFFICIENT_BUFFER)
+        *lpdwSize /= sizeof(WCHAR);
+
+Done:
+    if (name)
+        HeapFree( GetProcessHeap(), 0, name );
+
+    if (url)
+        HeapFree( GetProcessHeap(), 0, url );
+
+    if (szCookieData && szCookieData != &dummy)
+        HeapFree( GetProcessHeap(), 0, szCookieData );
 
     return r;
 }
diff --git a/dlls/wininet/tests/internet.c b/dlls/wininet/tests/internet.c
index 4da9c21..1144948 100644
--- a/dlls/wininet/tests/internet.c
+++ b/dlls/wininet/tests/internet.c
@@ -50,6 +50,25 @@ static int strcmp_ww(const WCHAR *str1, const WCHAR *str2)
     return memcmp(str1, str2, len1 * sizeof(WCHAR));
 }
 
+static const WCHAR *strstr_ww(const WCHAR *str, const WCHAR *sub)
+{
+    DWORD subLen = lstrlenW(sub) * sizeof(WCHAR);
+
+    while (*str)
+    {
+        /* found the start of the sub string => check if the string matches */
+        if (str[0] == sub[0])
+        {
+            if (!memcmp(str, sub, subLen))
+                return str;
+        }
+
+        str++;
+    }
+
+    return NULL;
+}
+
 /* ############################### */
 
 static void test_InternetCanonicalizeUrlA(void)
@@ -268,6 +287,20 @@ static void test_complicated_cookie(void)
   BOOL ret;
 
   CHAR buffer[1024];
+  WCHAR bufferw[1024];
+  WCHAR namew[] = {'C', 0};
+  WCHAR name2w[] = {'c', 0};
+  WCHAR cookieAw[] = {'A', '=', 'B', 0};
+  WCHAR cookieCw[] = {'C', '=', 'D', 0};
+  WCHAR cookieEw[] = {'E', '=', 'F', 0};
+  WCHAR cookieGw[] = {'G', '=', 'H', 0};
+  WCHAR cookieIw[] = {'I', '=', 'J', 0};
+  WCHAR cookieKw[] = {'K', '=', 'L', 0};
+  WCHAR cookieMw[] = {'M', '=', 'N', 0};
+  WCHAR cookieOw[] = {'O', '=', 'P', 0};
+  WCHAR urlw[] = {'h', 't', 't', 'p', ':', '/', '/', 't', 'e', 's', 't', 'i', 'n', 'g', '.',
+                  'e', 'x', 'a', 'm', 'p', 'l', 'e', '.', 'c', 'o', 'm', '/', 'b', 'a', 'r',
+                  '/', 'f', 'o', 'o', 0};
 
   ret = InternetSetCookie("http://www.example.com/bar",NULL,"A=B; domain=.example.com");
   ok(ret == TRUE,"InternetSetCookie failed\n");
@@ -364,6 +397,154 @@ static void test_complicated_cookie(void)
   ok(strstr(buffer,"K=L")!=NULL,"K=L missing\n");
   ok(strstr(buffer,"M=N")==NULL,"M=N present\n");
   ok(strstr(buffer,"O=P")==NULL,"O=P present\n");
+
+  /* test searching only for a specific cookie (should find all cookies still) */
+  len = 1024;
+  ret = InternetGetCookieA("http://testing.example.com/bar/foo", "C", buffer, &len);
+  ok(strstr(buffer,"A=B")!=NULL,"A=B missing\n");
+  ok(strstr(buffer,"C=D")!=NULL,"C=D missing\n");
+  ok(strstr(buffer,"E=F")!=NULL,"E=F missing\n");
+  ok(strstr(buffer,"G=H")==NULL,"G=H present\n");
+  ok(strstr(buffer,"I=J")!=NULL,"I=J missing\n");
+  ok(strstr(buffer,"K=L")!=NULL,"K=L missing\n");
+  ok(strstr(buffer,"M=N")==NULL,"M=N present\n");
+  ok(strstr(buffer,"O=P")==NULL,"O=P present\n");
+
+  /* test searching only for a specific cookie with the wrong case (should find all cookies still) */
+  len = 1024;
+  ret = InternetGetCookieA("http://testing.example.com/bar/foo", "c", buffer, &len);
+  ok(strstr(buffer,"A=B")!=NULL,"A=B missing\n");
+  ok(strstr(buffer,"C=D")!=NULL,"C=D missing\n");
+  ok(strstr(buffer,"E=F")!=NULL,"E=F missing\n");
+  ok(strstr(buffer,"G=H")==NULL,"G=H present\n");
+  ok(strstr(buffer,"I=J")!=NULL,"I=J missing\n");
+  ok(strstr(buffer,"K=L")!=NULL,"K=L missing\n");
+  ok(strstr(buffer,"M=N")==NULL,"M=N present\n");
+  ok(strstr(buffer,"O=P")==NULL,"O=P present\n");
+
+  /* test the wide version of the function */
+  len = 1024;
+  ret = InternetGetCookieW(urlw, NULL, bufferw, &len);
+  ok(strstr_ww(bufferw,cookieAw)!=NULL,"A=B missing\n");
+  ok(strstr_ww(bufferw,cookieCw)!=NULL,"C=D missing\n");
+  ok(strstr_ww(bufferw,cookieEw)!=NULL,"E=F missing\n");
+  ok(strstr_ww(bufferw,cookieGw)==NULL,"G=H present\n");
+  ok(strstr_ww(bufferw,cookieIw)!=NULL,"I=J missing\n");
+  ok(strstr_ww(bufferw,cookieKw)!=NULL,"K=L missing\n");
+  ok(strstr_ww(bufferw,cookieMw)==NULL,"M=N present\n");
+  ok(strstr_ww(bufferw,cookieOw)==NULL,"O=P present\n");
+
+  /* test searching only for a specific cookie (should only find the requested cookie) */
+  len = 1024;
+  ret = InternetGetCookieW(urlw, namew, bufferw, &len);
+  ok(strstr_ww(bufferw,cookieAw)==NULL,"A=B present\n");
+  ok(strstr_ww(bufferw,cookieCw)!=NULL,"C=D missing\n");
+  ok(strstr_ww(bufferw,cookieEw)==NULL,"E=F present\n");
+  ok(strstr_ww(bufferw,cookieGw)==NULL,"G=H present\n");
+  ok(strstr_ww(bufferw,cookieIw)==NULL,"I=J present\n");
+  ok(strstr_ww(bufferw,cookieKw)==NULL,"K=L present\n");
+  ok(strstr_ww(bufferw,cookieMw)==NULL,"M=N present\n");
+  ok(strstr_ww(bufferw,cookieOw)==NULL,"O=P present\n");
+
+  /* test searching only for a specific cookie with the wrong case (should only find the requested cookie) */
+  len = 1024;
+  ret = InternetGetCookieW(urlw, name2w, bufferw, &len);
+  ok(strstr_ww(bufferw,cookieAw)==NULL,"A=B present\n");
+  ok(strstr_ww(bufferw,cookieCw)!=NULL,"C=D missing\n");
+  ok(strstr_ww(bufferw,cookieEw)==NULL,"E=F present\n");
+  ok(strstr_ww(bufferw,cookieGw)==NULL,"G=H present\n");
+  ok(strstr_ww(bufferw,cookieIw)==NULL,"I=J present\n");
+  ok(strstr_ww(bufferw,cookieKw)==NULL,"K=L present\n");
+  ok(strstr_ww(bufferw,cookieMw)==NULL,"M=N present\n");
+  ok(strstr_ww(bufferw,cookieOw)==NULL,"O=P present\n");
+
+  /* test with a valid buffer and 0 length */
+  len = 0;
+  SetLastError(-1);
+  ret = InternetGetCookieA("http://testing.example.com/bar/foo", NULL, buffer, &len);
+  ok(!ret, "the function succeeded\n");
+  ok(GetLastError() == ERROR_INSUFFICIENT_BUFFER, "bad last error {error = %d}\n", GetLastError());
+  ok(len == 24, "unexpected required length {len = %d}\n", len);
+
+  /* test with a valid length but no buffer */
+  len = 1024;
+  SetLastError(-1);
+  ret = InternetGetCookieA("http://testing.example.com/bar/foo", NULL, NULL, &len);
+  ok(ret, "the function succeeded\n");
+  ok(GetLastError() == -1, "bad last error {error = %d}\n", GetLastError());
+  ok(len == 24, "unexpected required length {len = %d}\n", len);
+
+  /* test with a NULL length */
+  SetLastError(-1);
+  ret = InternetGetCookieA("http://testing.example.com/bar/foo", NULL, buffer, NULL);
+  ok(!ret, "the function succeeded\n");
+  ok(GetLastError() == ERROR_INVALID_PARAMETER, "bad last error {error = %d}\n", GetLastError());
+
+  /* test with a buffer that is too short */
+  len = 17;
+  SetLastError(-1);
+  ret = InternetGetCookieA("http://testing.example.com/bar/foo", NULL, buffer, &len);
+  ok(!ret, "the function succeeded\n");
+  ok(GetLastError() == ERROR_INSUFFICIENT_BUFFER, "bad last error {error = %d}\n", GetLastError());
+  ok(len == 24, "unexpected required length {len = %d}\n", len);
+
+  len = 23;
+  SetLastError(-1);
+  ret = InternetGetCookieA("http://testing.example.com/bar/foo", NULL, buffer, &len);
+  ok(!ret, "the function succeeded\n");
+  ok(GetLastError() == ERROR_INSUFFICIENT_BUFFER, "bad last error {error = %d}\n", GetLastError());
+  ok(len == 24, "unexpected required length {len = %d}\n", len);
+
+  len = 24;
+  SetLastError(-1);
+  ret = InternetGetCookieA("http://testing.example.com/bar/foo", NULL, buffer, &len);
+  ok(ret, "the function failed\n");
+  ok(GetLastError() == -1, "bad last error {error = %d}\n", GetLastError());
+  ok(len == 24, "unexpected required length {len = %d}\n", len);
+
+  len = 0;
+  SetLastError(-1);
+  ret = InternetGetCookieW(urlw, NULL, bufferw, &len);
+  ok(!ret, "the function succeeded\n");
+  ok(GetLastError() == ERROR_INSUFFICIENT_BUFFER, "bad last error {error = %d}\n", GetLastError());
+  ok(len == 48, "unexpected required length {len = %d}\n", len);
+
+  /* test with a valid length but no buffer */
+  len = 1024;
+  SetLastError(-1);
+  ret = InternetGetCookieW(urlw, NULL, NULL, &len);
+  ok(ret, "the function succeeded\n");
+  ok(GetLastError() == -1, "bad last error {error = %d}\n", GetLastError());
+  ok(len == 48, "unexpected required length {len = %d}\n", len);
+
+  /* test with a NULL length */
+  /* this test crashes on native */
+  /*SetLastError(-1);
+  ret = InternetGetCookieW(urlw, NULL, bufferw, NULL);
+  ok(!ret, "the function succeeded\n");
+  ok(GetLastError() == ERROR_INVALID_PARAMETER, "bad last error {error = %d}\n", GetLastError());*/
+
+  /* test with a buffer that is too short */
+  len = 17;
+  SetLastError(-1);
+  ret = InternetGetCookieW(urlw, NULL, bufferw, &len);
+  ok(!ret, "the function succeeded\n");
+  ok(GetLastError() == ERROR_INSUFFICIENT_BUFFER, "bad last error {error = %d}\n", GetLastError());
+  ok(len == 48, "unexpected required length {len = %d}\n", len);
+
+  len = 23;
+  SetLastError(-1);
+  ret = InternetGetCookieW(urlw, NULL, bufferw, &len);
+  ok(!ret, "the function succeeded\n");
+  ok(GetLastError() == ERROR_INSUFFICIENT_BUFFER, "bad last error {error = %d}\n", GetLastError());
+  ok(len == 48, "unexpected required length {len = %d}\n", len);
+
+  len = 24;
+  SetLastError(-1);
+  ret = InternetGetCookieW(urlw, NULL, bufferw, &len);
+  ok(ret, "the function failed\n");
+  ok(GetLastError() == -1, "bad last error {error = %d}\n", GetLastError());
+  ok(len == 24, "unexpected required length {len = %d}\n", len);
 }
 
 static void test_null(void)
-- 
1.6.6.1


--------------070506090407020705050909--



More information about the wine-patches mailing list