From: André Malo Date: Mon, 24 Nov 2003 21:34:38 +0000 (+0000) Subject: SECURITY [CAN-2003-0020]: escape arbitrary data before writing into the X-Git-Tag: pre_ajp_proxy~1011 X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=a7db87b9add29f7543457f53863cee4ee13d58c9;p=apache SECURITY [CAN-2003-0020]: escape arbitrary data before writing into the errorlog. Reviewed by: Mark J Cox git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/trunk@101873 13f79535-47bb-0310-9956-ffa450edef68 --- diff --git a/CHANGES b/CHANGES index fbbdc5e42f..6c9c887b93 100644 --- a/CHANGES +++ b/CHANGES @@ -2,6 +2,9 @@ Changes with Apache 2.1.0-dev [Remove entries to the current 2.0 section below, when backported] + *) SECURITY [CAN-2003-0020]: Escape arbitrary data before writing + into the errorlog. [André Malo] + *) mod_expires: Initialize ExpiresDefault to NULL instead of "" to avoid reporting an Internal Server error if it is used without having been set in the httpd.conf file. PR: 23748, 24459 diff --git a/include/ap_mmn.h b/include/ap_mmn.h index ef5d0aa121..1d912aea1a 100644 --- a/include/ap_mmn.h +++ b/include/ap_mmn.h @@ -117,6 +117,7 @@ * handler * 20030821 (2.1.0-dev) bumped mod_include's entire API * 20030821.1 (2.1.0-dev) added XHTML doctypes + * 20030821.2 (2.1.0-dev) added ap_escape_errorlog_item */ #define MODULE_MAGIC_COOKIE 0x41503230UL /* "AP20" */ @@ -124,7 +125,7 @@ #ifndef MODULE_MAGIC_NUMBER_MAJOR #define MODULE_MAGIC_NUMBER_MAJOR 20030821 #endif -#define MODULE_MAGIC_NUMBER_MINOR 1 /* 0...n */ +#define MODULE_MAGIC_NUMBER_MINOR 2 /* 0...n */ /** * Determine if the server's current MODULE_MAGIC_NUMBER is at least a diff --git a/include/httpd.h b/include/httpd.h index 175ce79de8..482a307930 100644 --- a/include/httpd.h +++ b/include/httpd.h @@ -1381,11 +1381,21 @@ AP_DECLARE(char *) ap_escape_html(apr_pool_t *p, const char *s); /** * Escape a string for logging * @param p The pool to allocate from - * @param s The string to escape + * @param str The string to escape * @return The escaped string */ AP_DECLARE(char *) ap_escape_logitem(apr_pool_t *p, const char *str); +/** + * Escape a string for logging into the error log (without a pool) + * @param dest The buffer to write to + * @param source The string to escape + * @param maxlen The buffer size for the escaped string (including \0) + * @return The len of the escaped string (always < maxlen) + */ +AP_DECLARE(apr_size_t) ap_escape_errorlog_item(char *dest, const char *source, + apr_size_t buflen); + /** * Construct a full hostname * @param p The pool to allocate from diff --git a/server/log.c b/server/log.c index d23753ee93..e01d10f45d 100644 --- a/server/log.c +++ b/server/log.c @@ -401,7 +401,7 @@ static void log_error_core(const char *file, int line, int level, const request_rec *r, apr_pool_t *pool, const char *fmt, va_list args) { - char errstr[MAX_STRING_LEN]; + char errstr[MAX_STRING_LEN], scratch[MAX_STRING_LEN]; apr_size_t len, errstrlen; apr_file_t *logf = NULL; const char *referer; @@ -536,12 +536,17 @@ static void log_error_core(const char *file, int line, int level, errstr[len] = '\0'; } } + errstrlen = len; - len += apr_vsnprintf(errstr + len, MAX_STRING_LEN - len, fmt, args); + if (apr_vsnprintf(scratch, MAX_STRING_LEN - len, fmt, args)) { + len += ap_escape_errorlog_item(errstr + len, scratch, + MAX_STRING_LEN - len); + } - if (r && (referer = apr_table_get(r->headers_in, "Referer"))) { + if ( r && (referer = apr_table_get(r->headers_in, "Referer")) + && ap_escape_errorlog_item(scratch, referer, MAX_STRING_LEN - len)) { len += apr_snprintf(errstr + len, MAX_STRING_LEN - len, - ", referer: %s", referer); + ", referer: %s", scratch); } /* NULL if we are logging to syslog */ diff --git a/server/util.c b/server/util.c index b83a8ca59e..8688bcf066 100644 --- a/server/util.c +++ b/server/util.c @@ -1838,6 +1838,70 @@ AP_DECLARE(char *) ap_escape_logitem(apr_pool_t *p, const char *str) return ret; } +AP_DECLARE(apr_size_t) ap_escape_errorlog_item(char *dest, const char *source, + apr_size_t buflen) +{ + unsigned char *d, *ep; + const unsigned char *s; + + if (!source || !buflen) { /* be safe */ + return 0; + } + + d = (unsigned char *)dest; + s = (const unsigned char *)source; + ep = d + buflen - 1; + + for (; d < ep && *s; ++s) { + + if (TEST_CHAR(*s, T_ESCAPE_LOGITEM)) { + *d++ = '\\'; + if (d >= ep) { + --d; + break; + } + + switch(*s) { + case '\b': + *d++ = 'b'; + break; + case '\n': + *d++ = 'n'; + break; + case '\r': + *d++ = 'r'; + break; + case '\t': + *d++ = 't'; + break; + case '\v': + *d++ = 'v'; + break; + case '\\': + *d++ = *s; + break; + case '"': /* no need for this in error log */ + d[-1] = *s; + break; + default: + if (d >= ep - 2) { + ep = --d; /* break the for loop as well */ + break; + } + c2x(*s, d); + *d = 'x'; + d += 3; + } + } + else { + *d++ = *s; + } + } + *d = '\0'; + + return (d - (unsigned char *)dest); +} + AP_DECLARE(int) ap_is_directory(apr_pool_t *p, const char *path) { apr_finfo_t finfo;