You are viewing a plain text version of this content. The canonical link for it is here.
Posted to cvs@httpd.apache.org by nd...@apache.org on 2003/12/14 19:16:50 UTC
cvs commit: apache-1.3/src/main http_log.c util.c
nd 2003/12/14 10:16:50
Modified: src CHANGES
src/include ap_mmn.h httpd.h
src/main http_log.c util.c
Log:
SECURITY [CAN-2003-0020]: escape arbitrary data before writing into the
errorlog.
Reviewed by: Mark J Cox, Erik Abele, Jeff Trawick
Revision Changes Path
1.1914 +3 -0 apache-1.3/src/CHANGES
Index: CHANGES
===================================================================
RCS file: /home/cvs/apache-1.3/src/CHANGES,v
retrieving revision 1.1913
retrieving revision 1.1914
diff -u -u -r1.1913 -r1.1914
--- CHANGES 4 Dec 2003 14:44:49 -0000 1.1913
+++ CHANGES 14 Dec 2003 18:16:49 -0000 1.1914
@@ -1,5 +1,8 @@
Changes with Apache 1.3.30
+ *) SECURITY [CAN-2003-0020]: Escape arbitrary data before writing
+ into the errorlog. [Andr� Malo]
+
*) '%X' is now accepted as an alias for '%c' in the
LogFormat directive. This allows you to configure logging
to still log the connection status even with mod_ssl
1.65 +2 -1 apache-1.3/src/include/ap_mmn.h
Index: ap_mmn.h
===================================================================
RCS file: /home/cvs/apache-1.3/src/include/ap_mmn.h,v
retrieving revision 1.64
retrieving revision 1.65
diff -u -u -r1.64 -r1.65
--- ap_mmn.h 7 Jul 2003 00:34:09 -0000 1.64
+++ ap_mmn.h 14 Dec 2003 18:16:49 -0000 1.65
@@ -243,6 +243,7 @@
* ap_note_cleanups_for_file_ex(),
* ap_popenf_ex() and ap_psocket_ex().
* 19990320.15 - ap_is_recursion_limit_exceeded()
+ * 19990320.16 - ap_escape_errorlog_item()
*/
#define MODULE_MAGIC_COOKIE 0x41503133UL /* "AP13" */
@@ -250,7 +251,7 @@
#ifndef MODULE_MAGIC_NUMBER_MAJOR
#define MODULE_MAGIC_NUMBER_MAJOR 19990320
#endif
-#define MODULE_MAGIC_NUMBER_MINOR 15 /* 0...n */
+#define MODULE_MAGIC_NUMBER_MINOR 16 /* 0...n */
/* Useful for testing for features. */
#define AP_MODULE_MAGIC_AT_LEAST(major,minor) \
1.374 +2 -0 apache-1.3/src/include/httpd.h
Index: httpd.h
===================================================================
RCS file: /home/cvs/apache-1.3/src/include/httpd.h,v
retrieving revision 1.373
retrieving revision 1.374
diff -u -u -r1.373 -r1.374
--- httpd.h 24 Oct 2003 16:18:00 -0000 1.373
+++ httpd.h 14 Dec 2003 18:16:49 -0000 1.374
@@ -1028,6 +1028,8 @@
API_EXPORT(char *) ap_construct_server(pool *p, const char *hostname,
unsigned port, const request_rec *r);
API_EXPORT(char *) ap_escape_logitem(pool *p, const char *str);
+API_EXPORT(size_t) ap_escape_errorlog_item(char *dest, const char *source,
+ size_t buflen);
API_EXPORT(char *) ap_escape_shell_cmd(pool *p, const char *s);
API_EXPORT(int) ap_count_dirs(const char *path);
1.97 +5 -2 apache-1.3/src/main/http_log.c
Index: http_log.c
===================================================================
RCS file: /home/cvs/apache-1.3/src/main/http_log.c,v
retrieving revision 1.96
retrieving revision 1.97
diff -u -u -r1.96 -r1.97
--- http_log.c 3 Feb 2003 17:13:21 -0000 1.96
+++ http_log.c 14 Dec 2003 18:16:50 -0000 1.97
@@ -313,7 +313,7 @@
const server_rec *s, const request_rec *r,
const char *fmt, va_list args)
{
- char errstr[MAX_STRING_LEN];
+ char errstr[MAX_STRING_LEN], scratch[MAX_STRING_LEN];
size_t len;
int save_errno = errno;
FILE *logf;
@@ -445,7 +445,10 @@
}
#endif
- len += ap_vsnprintf(errstr + len, sizeof(errstr) - len, fmt, args);
+ if (ap_vsnprintf(scratch, sizeof(scratch) - len, fmt, args)) {
+ len += ap_escape_errorlog_item(errstr + len, scratch,
+ sizeof(errstr) - len);
+ }
/* NULL if we are logging to syslog */
if (logf) {
1.208 +63 -0 apache-1.3/src/main/util.c
Index: util.c
===================================================================
RCS file: /home/cvs/apache-1.3/src/main/util.c,v
retrieving revision 1.207
retrieving revision 1.208
diff -u -u -r1.207 -r1.208
--- util.c 3 Feb 2003 17:13:23 -0000 1.207
+++ util.c 14 Dec 2003 18:16:50 -0000 1.208
@@ -1520,6 +1520,69 @@
return ret;
}
+API_EXPORT(size_t) ap_escape_errorlog_item(char *dest, const char *source,
+ 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);
+}
API_EXPORT(char *) ap_escape_shell_cmd(pool *p, const char *str)
{
Re: cvs commit: apache-1.3/src/main http_log.c util.c
Posted by André Malo <nd...@perlig.de>.
* Ben Laurie <be...@algroup.co.uk> wrote:
> > - char errstr[MAX_STRING_LEN];
> > + char errstr[MAX_STRING_LEN], scratch[MAX_STRING_LEN];
>
> Surely scratch and errstr should be MAX_STRING_LEN*4?
Hmm. MAX_STRING_LEN is 8k. So we put 16k onto the stack now. Would you
really like to mess up the stack with 64KB for error message stuff? At least
on threaded platforms this will fail.
> > + c2x(*s, d);
>
> Am I being dim? Shouldn't this be c2x(*s,d+1)?
ehm, no?
nd
Re: cvs commit: apache-1.3/src/main http_log.c util.c
Posted by Ben Laurie <be...@algroup.co.uk>.
nd@apache.org wrote:
> nd 2003/12/14 10:16:50
>
> Modified: src CHANGES
> src/include ap_mmn.h httpd.h
> src/main http_log.c util.c
> Log:
> SECURITY [CAN-2003-0020]: escape arbitrary data before writing into the
> errorlog.
> Index: http_log.c
> ===================================================================
> RCS file: /home/cvs/apache-1.3/src/main/http_log.c,v
> retrieving revision 1.96
> retrieving revision 1.97
> diff -u -u -r1.96 -r1.97
> --- http_log.c 3 Feb 2003 17:13:21 -0000 1.96
> +++ http_log.c 14 Dec 2003 18:16:50 -0000 1.97
> @@ -313,7 +313,7 @@
> const server_rec *s, const request_rec *r,
> const char *fmt, va_list args)
> {
> - char errstr[MAX_STRING_LEN];
> + char errstr[MAX_STRING_LEN], scratch[MAX_STRING_LEN];
Surely scratch and errstr should be MAX_STRING_LEN*4?
> + default:
> + if (d >= ep - 2) {
> + ep = --d; /* break the for loop as well */
> + break;
> + }
> + c2x(*s, d);
Am I being dim? Shouldn't this be c2x(*s,d+1)?
> + *d = 'x';
> + d += 3;
> + }
> + }
> + else {
> + *d++ = *s;
> + }
> + }
> + *d = '\0';
> +
> + return (d - (unsigned char *)dest);
> +}
Cheers,
Ben.
--
http://www.apache-ssl.org/ben.html http://www.thebunker.net/
"There is no limit to what a man can do or how far he can go if he
doesn't mind who gets the credit." - Robert Woodruff
Re: cvs commit: apache-1.3/src/main http_log.c util.c
Posted by Ben Laurie <be...@algroup.co.uk>.
nd@apache.org wrote:
> nd 2003/12/14 10:16:50
>
> Modified: src CHANGES
> src/include ap_mmn.h httpd.h
> src/main http_log.c util.c
> Log:
> SECURITY [CAN-2003-0020]: escape arbitrary data before writing into the
> errorlog.
> Index: http_log.c
> ===================================================================
> RCS file: /home/cvs/apache-1.3/src/main/http_log.c,v
> retrieving revision 1.96
> retrieving revision 1.97
> diff -u -u -r1.96 -r1.97
> --- http_log.c 3 Feb 2003 17:13:21 -0000 1.96
> +++ http_log.c 14 Dec 2003 18:16:50 -0000 1.97
> @@ -313,7 +313,7 @@
> const server_rec *s, const request_rec *r,
> const char *fmt, va_list args)
> {
> - char errstr[MAX_STRING_LEN];
> + char errstr[MAX_STRING_LEN], scratch[MAX_STRING_LEN];
Surely scratch and errstr should be MAX_STRING_LEN*4?
> + default:
> + if (d >= ep - 2) {
> + ep = --d; /* break the for loop as well */
> + break;
> + }
> + c2x(*s, d);
Am I being dim? Shouldn't this be c2x(*s,d+1)?
> + *d = 'x';
> + d += 3;
> + }
> + }
> + else {
> + *d++ = *s;
> + }
> + }
> + *d = '\0';
> +
> + return (d - (unsigned char *)dest);
> +}
Cheers,
Ben.
--
http://www.apache-ssl.org/ben.html http://www.thebunker.net/
"There is no limit to what a man can do or how far he can go if he
doesn't mind who gets the credit." - Robert Woodruff