#25 Vary header is sometimes truncated

mod_gzip-1.3.26.1a
open
nobody
None
5
2009-01-10
2009-01-10
Ethan Fischer
No

When mod_gzip_send_vary is "on" and a reqheader is defined, mod_gzip will send a Vary header to apprise web caches that there may be different versions of the file available.

The code which generates Vary headers for reqheader entries uses an uninitialized character array, which interacts badly with a broken implementation of mod_gzip_strncpy. The result is that the Vary header is sometimes truncated.

The problem is on line 1232 of mod_gzip-1.3.26.1a/mod_gzip.c, in mod_gzip_strncpy:

while(*s2 != 0 && *s1 != 0 && len <= l) {

This line checks both the source and target strings for null bytes. However, the target string is unintialized and allocated from the stack, having a high percentage of random zeroes in it. This causes the copy to stop partway through sometimes.

The easy way around this problem is to simply turn mod_gzip_send_vary off.

There are several ways to actually fix the problem. The way I prefer is to use the libc strncpy and check the length of the string we're copying to avoid overflow, like so:

char *mod_gzip_generate_vary_header(mod_gzip_conf *cfg,struct pool *p) {
int i = 0;
int len = 0;
char name[MOD_GZIP_IMAP_MAXNAMELEN + 2];
array_header *ary = ap_make_array(p,cfg->imap_total_isreqheader+1,sizeof(char *));

*((const char **)ap_push_array(ary)) = ap_pstrdup(p,"Accept-Encoding");

for(i=0;i<cfg->imap_total_entries;i++) {
if(cfg->imap[i].type == MOD_GZIP_IMAP_ISREQHEADER) {
len = strstr(cfg->imap[i].name,":") - cfg->imap[i].name - 1;
if (len > MOD_GZIP_IMAP_MAXNAMELEN) {
/* Buffer is too small. Prefer to cache too many copies than send a broken header. */
continue;
}
strncpy(name,cfg->imap[i].name,len);
name[len] = '\0';
*((const char **)ap_push_array(ary)) = ap_pstrdup(p,name);
}
}

return ap_array_pstrcat(p,ary,',');
}

Discussion