On 11/02/2014 05:09 PM, Yann Ylavic wrote:
Hi,
On Thu, Sep 4, 2014 at 12:52 PM, jkal...@apache.org wrote:
Author: jkaluza
Date: Thu Sep 4 10:52:24 2014
New Revision: 1622450
URL: http://svn.apache.org/r1622450
Log:
ab: increase request and response header size to 8192 bytes,
fix potential buffer-overflow in Server: header handling.
Modified:
httpd/httpd/trunk/support/ab.c
Modified: httpd/httpd/trunk/support/ab.c
URL:
http://svn.apache.org/viewvc/httpd/httpd/trunk/support/ab.c?rev=1622450r1=1622449r2=1622450view=diff
==
--- httpd/httpd/trunk/support/ab.c (original)
+++ httpd/httpd/trunk/support/ab.c Thu Sep 4 10:52:24 2014
[snip]
@@ -1516,12 +1516,14 @@ static void read_connection(struct conne
* this is first time, extract some interesting info
*/
char *p, *q;
+size_t len = 0;
p = strstr(c-cbuff, Server:);
q = servername;
if (p) {
p += 8;
-while (*p 32)
-*q++ = *p++;
+/* -1 to not overwrite last '\0' byte */
+while (*p 32 len++ sizeof(servername) - 1)
Maybe ++len above (instead of len++) since we need to leave room for
the final '\0' below?
Otherwise we may still overflow when writing it to
servername[sizeof(servername)]...
I think technically that code is OK. It writes sizeof(servername) - 1
characters to servername and keeps the last byte for zero. It could be
rewritten as ++len sizeof(servername), but the result is the same
and since gcc optimizes that, it even generates the same code.
Just to be really sure, I wrote following test code:
#include stdio.h
#include stdlib.h
#define BUFF_SIZE 10
int main(int argc, char **argv) {
char *servername = malloc(BUFF_SIZE);
char original[] = Something_longer_than_10_bytes;
char *p = original, *q = servername;
size_t len = 0;
while (*p 32 len++ BUFF_SIZE - 1)
*q++ = *p++;
*q = 0;
printf('%s'\n, servername);
return 0;
}
Running that in valgrind looks OK too.
$ gcc test.c
$ valgrind -q ./a.out
'Something'
$
Am I missing something?
Regards,
Jan Kaluza
+*q++ = *p++;
}
*q = 0;
}
Regards,
Yann.
