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=1622450&r1=1622449&r2=1622450&view=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.
