Potential buffer overflow in CHttpRequestSender::InternalSend()

[GoogleCodeExporter]

When reading the response:

    InternetReadFile(hRequest, pBuffer, 2048, &dwBuffSize);
    pBuffer[dwBuffSize] = 0;

and the response is longer than 2047 characters, dwBuffSize will be 2048 and 
the second line will write beyond the allocated buffer. In addition it would be 
preferable to read the whole response (for me it was important to actually 
write it to a file which can be opened by the browser) to get potential 
debugging information from a server.
In the same context it seems preferable to use the status of the request 
response instead of reading the string to decide whether sending was successful.
I use:

  DWORD lStatus;
  DWORD lStatusSize = sizeof(lStatus);
  HttpQueryInfo(hRequest, HTTP_QUERY_STATUS_CODE|HTTP_QUERY_FLAG_NUMBER, &lStatus, &lStatusSize, 0);

  if(lStatus!=200)
  {
    ...

A patched HttpRequestSender.cpp is attached.

Original issue reported on code.google.com by asch...@gwdg.de on 30 Jun 2011 at 9:57

Attachments:

• HttpRequestSender.cpp

[GoogleCodeExporter]

Original comment by zexspect...@gmail.com on 31 Jul 2011 at 3:39

• Changed state: Reviewed

[GoogleCodeExporter]

I can fix the potential overflow and increase buffer size to, say, 4096 bytes, 
but I can't change the way it reads responce because of backwards 
compatibility. 

Original comment by zexspect...@gmail.com on 9 Oct 2011 at 3:43

• Changed state: Accepted

[GoogleCodeExporter]

Thanks for looking into it. Of course the overflow is the most important issue 
it caused crashes whenever debugging the server as it then writes lengthy 
status information into the response.
I understand that getting the '200 OK' in the string still needs to indicate 
success as not to break existing server implementations. 
However, would it be possible to allow an lStatus of 200 to also indicate 
success? Maybe as an optional feature? It would make life easier in case some 
debugging or status information is written back from the server.
Also, while reading the whole response is certainly not necessary in any of the 
scenarios, the option of doing this and writing it to a file location may be a 
useful feature for debugging a server implementation. 
Would you consider including it if I sent you a patch?

Original comment by Schoenle...@googlemail.com on 10 Oct 2011 at 7:11

[GoogleCodeExporter]

Ok, I'll try to implement what you ask. Your patch will be appreciated.

Original comment by zexspect...@gmail.com on 10 Oct 2011 at 7:58

[GoogleCodeExporter]

Fixed in v.1.3.0

Original comment by zexspect...@gmail.com on 22 Oct 2011 at 11:04

[GoogleCodeExporter]

Original comment by zexspect...@gmail.com on 22 Oct 2011 at 11:04

• Changed state: Fixed