Re: Possible security problem in referer_log_transaction... ? (fwd)

[Dean Gaudet <dg...@hotwired.com>]

In HotWired's server I use a hacked up mod_log_common called
mod_log_unified.  It's more or less identical except for the actual
routine which writes the log entry.  I deal with:

- unprintable characters
- use of field delimiters in client-supplied data
- limiting length of client-supplied data

Sorry, it's not a patch.

It also assumes that sprintf returns the number of bytes written.
Just say NO to lame extern char *sprintf();.  But you might find the
sanitize_string routine useful.

Dean

/* I've used this routine under this name since I learned how to program in
    C under Lattice C for the Amiga.  I don't know if they're the original
    source of the routine... but anyhow, it's strcpy that returns a
    pointer to the zero byte of dest, which is way more useful than
    returning dest itself. -djg */

char *stpcpy( char *dest, const char *src )
{
    while( *dest = *src ) {
	++dest;
	++src;
    }
    return( dest );
}

char *sanitize_string( char *dest, const char *src, size_t len )
{
    static const char hex[] = "0123456789ABCDEF";
    char *d;

    d = dest;
    while( *src ) {
	if( !isprint( *src ) || *src == '"' || *src == '%' || *src == '*' ) {
	    /* use % escape to write it, we need at least 3 characters to
		do the escape, and possibly one more if there's more to src */
	    if( d - dest >= len - 3 - ( src[1] != 0 ) ) break;
	    *d++ = '%';
	    *d++ = hex[ (unsigned)*src / 16 ];
	    *d++ = hex[ (unsigned)*src % 16 ];
	} else {
	    if( d - dest >= len - 1 - ( src[1] != 0 ) ) break;
	    *d++ = *src;
	}
	++src;
    }
    if( *src ) {
	*d++ = '*';
    }
    *d = 0;
    return( d );
}


int unified_log_transaction(request_rec *orig)
{
    unified_log_state *cls = get_module_config (orig->server->module_config,
					       &unified_log_module);

    char str[HUGE_STRING_LEN];
    long timz;
    struct tm *t;
    char sign;
    char tstr[MAX_STRING_LEN];
    conn_rec *c = orig->connection;
    request_rec *r;
    char *p;
    char *header;
    size_t l;

    /* Common log format records an unholy melange of the original request
     * and whatever it was that we actually served.  Just stay compatible
     * here; the whole point of the module scheme is to allow people to
     * create better alternatives, but screwing up is an option we wish
     * to preserve...
     */

    for (r = orig; r->next; r = r->next)
        continue;

    t = get_gmtoff(&timz);
    sign = (timz < 0 ? '-' : '+');
    if(timz < 0) 
        timz = -timz;

    strftime(tstr,MAX_STRING_LEN,"%d/%b/%Y:%H:%M:%S",t);

    p = stpcpy( str, c->remote_name );
    *p++ = ' ';
    p = stpcpy( p, (c->remote_logname ? c->remote_logname : "-") );
    *p++ = ' ';
    p = stpcpy( p, (c->user ? c->user : "-") );
    *p++ = ' ';
    *p++ = '[';
    p = stpcpy( p, tstr );
    *p++ = ' ';
    *p++ = sign;
    p += sprintf( p, "%02ld%02ld] \"", timz/3600, timz%3600 );
    if( orig->the_request ) p = sanitize_string( p, orig->the_request, 512 );
    *p++ = '"';
    *p++ = ' ';

    if (r->status != -1) {
        p += sprintf(p,"%d ",r->status);
    } else {
	*p++ = '-';
	*p++ = ' ';
    }

    if(r->bytes_sent != -1) {
        p += sprintf(p,"%d ",r->bytes_sent);
    } else {
	*p++ = '-';
	*p++ = ' ';
    }

    header = table_get( orig->headers_in, "Referer" );
    if( header ) {
	l = strlen( r->uri );
	/* we don't want to record refers due to <IMG> tags */
	if( ( l >= 4 && strcmp( r->uri + l - 4, ".gif" ) == 0 )
	    || ( l >= 4 && strcmp( r->uri + l - 4, ".jpg" ) == 0 )
	    || ( l >= 5 && strcmp( r->uri + l - 5, ".jpeg" ) == 0 ) ) {
	    header = NULL;
	} else if( strncmp( header, "file:", 5 ) == 0 ) {
	    header = NULL;
	}
    }
    if( header ) {
	*p++ = '"';
	p = sanitize_string( p, header, 512 );
	*p++ = '"';
    } else {
	*p++ = '-';
    }
    *p++ = ' ';
#if 0
    header = table_get( orig->headers_in, "User-Agent" );
    if( header ) {
	*p++ = '"';
	p = sanitize_string( p, header, 512 );
	*p++ = '"';
    } else {
	*p++ = '-';
    }
#else
    *p++ = '-';
#endif
    *p++ = '\n';

    write(cls->log_fd, str, p - str);

    return OK;
}