Use Heap Allocated Buffers in HTTP_InterpretHTTPHeader
Robert Shearman
rob at codeweavers.com
Thu Sep 23 07:53:17 CDT 2004
Hi,
The previous code had two problems:
1. Using a fixed sized buffer on the stack meaning that it couldn't
process large headers and could be a potential security risk if it
overflowed.
2. Buffer overflow on large data.
As the code shouldn't have been using fixed sized buffers I changed it
to dynamically allocate buffers for it which can then be freed using the
token API, rather than just fixing the buffer overflow.
Rob
Changelog:
Use dynamically allocated heap allocated buffers in
HTTP_InterpretHTTPHeader to avoid buffer overflow on large headers.
-------------- next part --------------
Index: wine/dlls/wininet/http.c
===================================================================
RCS file: /home/wine/wine/dlls/wininet/http.c,v
retrieving revision 1.85
diff -u -p -r1.85 http.c
--- wine/dlls/wininet/http.c 20 Sep 2004 21:43:47 -0000 1.85
+++ wine/dlls/wininet/http.c 23 Sep 2004 12:43:19 -0000
@@ -93,7 +93,7 @@ BOOL HTTP_GetResponseHeaders(LPWININETHT
BOOL HTTP_ProcessHeader(LPWININETHTTPREQW lpwhr, LPCWSTR field, LPCWSTR value, DWORD dwModifier);
BOOL HTTP_ReplaceHeaderValue( LPHTTPHEADERW lphttpHdr, LPCWSTR lpsztmp );
void HTTP_CloseConnection(LPWININETHTTPREQW lpwhr);
-BOOL HTTP_InterpretHttpHeader(LPWSTR buffer, LPWSTR field, INT fieldlen, LPWSTR value, INT valuelen);
+LPWSTR * HTTP_InterpretHttpHeader(LPCWSTR buffer);
INT HTTP_GetStdHeaderIndex(LPCWSTR lpszField);
BOOL HTTP_InsertCustomHeader(LPWININETHTTPREQW lpwhr, LPHTTPHEADERW lpHdr);
INT HTTP_GetCustomHeaderIndex(LPWININETHTTPREQW lpwhr, LPCWSTR lpszField);
@@ -168,7 +168,6 @@ static BOOL WINAPI HTTP_HttpAddRequestHe
LPWSTR lpszStart;
LPWSTR lpszEnd;
LPWSTR buffer;
- WCHAR value[MAX_FIELD_VALUE_LEN], field[MAX_FIELD_LEN];
BOOL bSuccess = FALSE;
DWORD len;
@@ -186,6 +185,8 @@ static BOOL WINAPI HTTP_HttpAddRequestHe
do
{
+ LPWSTR * pFieldAndValue;
+
lpszEnd = lpszStart;
while (*lpszEnd != '\0')
@@ -204,11 +205,15 @@ static BOOL WINAPI HTTP_HttpAddRequestHe
lpszEnd += 2; /* Jump over \r\n */
}
TRACE("interpreting header %s\n", debugstr_w(lpszStart));
- if (HTTP_InterpretHttpHeader(lpszStart, field, MAX_FIELD_LEN, value, MAX_FIELD_VALUE_LEN))
- bSuccess = HTTP_ProcessHeader(lpwhr, field, value, dwModifier | HTTP_ADDHDR_FLAG_REQ);
+ pFieldAndValue = HTTP_InterpretHttpHeader(lpszStart);
+ if (pFieldAndValue)
+ {
+ bSuccess = HTTP_ProcessHeader(lpwhr, pFieldAndValue[0],
+ pFieldAndValue[1], dwModifier | HTTP_ADDHDR_FLAG_REQ);
+ HTTP_FreeTokens(pFieldAndValue);
+ }
lpszStart = lpszEnd;
-
} while (bSuccess);
HeapFree(GetProcessHeap(), 0, buffer);
@@ -1981,7 +1986,6 @@ BOOL HTTP_GetResponseHeaders(LPWININETHT
DWORD buflen = MAX_REPLY_LEN;
BOOL bSuccess = FALSE;
INT rc = 0;
- WCHAR value[MAX_FIELD_VALUE_LEN], field[MAX_FIELD_LEN];
static const WCHAR szCrLf[] = {'\r','\n',0};
char bufferA[MAX_REPLY_LEN];
LPWSTR status_code, status_text;
@@ -2046,7 +2050,9 @@ BOOL HTTP_GetResponseHeaders(LPWININETHT
{
buflen = MAX_REPLY_LEN;
if (NETCON_getNextLine(&lpwhr->netConnection, bufferA, &buflen))
- {
+ {
+ LPWSTR * pFieldAndValue;
+
TRACE("got line %s, now interpretting\n", debugstr_a(bufferA));
MultiByteToWideChar( CP_ACP, 0, bufferA, buflen, buffer, MAX_REPLY_LEN );
@@ -2061,10 +2067,14 @@ BOOL HTTP_GetResponseHeaders(LPWININETHT
cchRawHeaders += sizeof(szCrLf)/sizeof(szCrLf[0])-1;
lpszRawHeaders[cchRawHeaders] = '\0';
- if (!HTTP_InterpretHttpHeader(buffer, field, MAX_FIELD_LEN, value, MAX_FIELD_VALUE_LEN))
+ pFieldAndValue = HTTP_InterpretHttpHeader(buffer);
+ if (!pFieldAndValue)
break;
- HTTP_ProcessHeader(lpwhr, field, value, (HTTP_ADDREQ_FLAG_ADD | HTTP_ADDREQ_FLAG_REPLACE));
+ HTTP_ProcessHeader(lpwhr, pFieldAndValue[0], pFieldAndValue[1],
+ HTTP_ADDREQ_FLAG_ADD | HTTP_ADDREQ_FLAG_REPLACE);
+
+ HTTP_FreeTokens(pFieldAndValue);
}
else
{
@@ -2089,6 +2099,26 @@ lend:
}
+static void strip_spaces(LPWSTR start)
+{
+ LPWSTR str = start;
+ LPWSTR end;
+
+ while (*str == ' ' && *str != '\0')
+ str++;
+
+ if (str != start)
+ memmove(start, str, sizeof(WCHAR) * (strlenW(str) + 1));
+
+ end = start + strlenW(start) - 1;
+ while(*end == ' ' && *end != '\0')
+ {
+ *end = '\0';
+ end--;
+ }
+}
+
+
/***********************************************************************
* HTTP_InterpretHttpHeader (internal)
*
@@ -2096,59 +2126,50 @@ lend:
*
* RETURNS
*
- * TRUE on success
- * FALSE on error
+ * Pointer to array of field, value, NULL on success.
+ * NULL on error.
*/
-static INT stripSpaces(LPCWSTR lpszSrc, LPWSTR lpszStart, INT *len)
+LPWSTR * HTTP_InterpretHttpHeader(LPCWSTR buffer)
{
- LPCWSTR lpsztmp;
- INT srclen;
-
- srclen = 0;
+ LPWSTR * pTokenPair;
+ LPWSTR pszColon;
+ INT len;
- while (*lpszSrc == ' ' && *lpszSrc != '\0')
- lpszSrc++;
+ pTokenPair = HeapAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY, sizeof(*pTokenPair)*3);
- lpsztmp = lpszSrc;
- while(*lpsztmp != '\0')
+ pszColon = strchrW(buffer, ':');
+ /* must have two tokens */
+ if (!pszColon)
{
- if (*lpsztmp != ' ')
- srclen = lpsztmp - lpszSrc + 1;
-
- lpsztmp++;
+ HTTP_FreeTokens(pTokenPair);
+ return NULL;
}
- *len = min(*len, srclen);
- strncpyW(lpszStart, lpszSrc, *len);
- lpszStart[*len] = '\0';
-
- return *len;
-}
-
-
-BOOL HTTP_InterpretHttpHeader(LPWSTR buffer, LPWSTR field, INT fieldlen, LPWSTR value, INT valuelen)
-{
- WCHAR *pd;
- BOOL bSuccess = FALSE;
-
- TRACE("\n");
-
- *field = '\0';
- *value = '\0';
+ pTokenPair[0] = HeapAlloc(GetProcessHeap(), 0, (pszColon - buffer + 1) * sizeof(WCHAR));
+ if (!pTokenPair[0])
+ {
+ HTTP_FreeTokens(pTokenPair);
+ return NULL;
+ }
+ memcpy(pTokenPair[0], buffer, (pszColon - buffer) * sizeof(WCHAR));
+ pTokenPair[0][pszColon - buffer] = '\0';
- pd = strchrW(buffer, ':');
- if (pd)
+ /* skip colon */
+ pszColon++;
+ len = strlenW(pszColon);
+ pTokenPair[1] = HeapAlloc(GetProcessHeap(), 0, (len + 1) * sizeof(WCHAR));
+ if (!pTokenPair[1])
{
- *pd = '\0';
- if (stripSpaces(buffer, field, &fieldlen) > 0)
- {
- if (stripSpaces(pd+1, value, &valuelen) > 0)
- bSuccess = TRUE;
- }
+ HTTP_FreeTokens(pTokenPair);
+ return NULL;
}
+ memcpy(pTokenPair[1], pszColon, (len + 1) * sizeof(WCHAR));
- TRACE("%d: field(%s) Value(%s)\n", bSuccess, debugstr_w(field), debugstr_w(value));
- return bSuccess;
+ strip_spaces(pTokenPair[0]);
+ strip_spaces(pTokenPair[1]);
+
+ TRACE("field(%s) Value(%s)\n", debugstr_w(pTokenPair[0]), debugstr_w(pTokenPair[1]));
+ return pTokenPair;
}
@@ -2530,7 +2551,7 @@ INT HTTP_GetCustomHeaderIndex(LPWININETH
if (index >= lpwhr->nCustHeaders)
index = -1;
- TRACE("Return: %lu\n", index);
+ TRACE("Return: %ld\n", index);
return index;
}
More information about the wine-patches
mailing list