You are viewing a plain text version of this content. The canonical link for it is here.
Posted to cvs@httpd.apache.org by ma...@apache.org on 2002/05/21 15:03:56 UTC
cvs commit: apache-1.3/src/support httpd.exp
martin 02/05/21 06:03:56
Modified: src CHANGES
src/include httpd.h
src/main gen_test_char.c http_protocol.c util.c
src/modules/standard mod_log_config.c
src/support httpd.exp
Log:
Apply a stricter check to the request line syntax, in order to prevent
arbitrary user input to end up (unescaped) in the access_log and error_log
files. Until now, garbage could be injected to spoof accesses to nonexistent
(or inaccessible) resources -- of course without the client actually
getting access to them.
Now anything but whitespace following the "<method> <url> HTTP/x.y" request
line is disallowed, and special characters in the request are escaped
in the log.
Revision Changes Path
1.1822 +8 -0 apache-1.3/src/CHANGES
Index: CHANGES
===================================================================
RCS file: /home/cvs/apache-1.3/src/CHANGES,v
retrieving revision 1.1821
retrieving revision 1.1822
diff -u -r1.1821 -r1.1822
--- CHANGES 21 May 2002 12:24:58 -0000 1.1821
+++ CHANGES 21 May 2002 13:03:55 -0000 1.1822
@@ -1,5 +1,13 @@
Changes with Apache 1.3.25
+ *) Disallow anything but whitespace on the request line after the
+ HTTP/x.y protocol string. That prevents arbitrary user input
+ from ending up in the access_log and error_log. Also, special
+ characters (especially control characters) are escaped in the
+ log file now, to make a clear distinction between client-supplied
+ strings (with special characters) and server-side strings.
+ [Martin Kraemer]
+
*) Get rid of DEFAULT_XFERLOG as it is not used anywhere. It was
preserved by the build system, printed with "httpd -V", but
apart from that completely ignored: the default transfer log
1.361 +1 -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.360
retrieving revision 1.361
diff -u -r1.360 -r1.361
--- httpd.h 21 May 2002 12:24:59 -0000 1.360
+++ httpd.h 21 May 2002 13:03:55 -0000 1.361
@@ -1023,6 +1023,7 @@
API_EXPORT(char *) ap_escape_html(pool *p, const char *s);
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(char *) ap_escape_shell_cmd(pool *p, const char *s);
API_EXPORT(int) ap_count_dirs(const char *path);
1.8 +25 -11 apache-1.3/src/main/gen_test_char.c
Index: gen_test_char.c
===================================================================
RCS file: /home/cvs/apache-1.3/src/main/gen_test_char.c,v
retrieving revision 1.7
retrieving revision 1.8
diff -u -r1.7 -r1.8
--- gen_test_char.c 21 Mar 2002 16:02:03 -0000 1.7
+++ gen_test_char.c 21 May 2002 13:03:56 -0000 1.8
@@ -8,6 +8,7 @@
#define T_ESCAPE_PATH_SEGMENT (0x02)
#define T_OS_ESCAPE_PATH (0x04)
#define T_HTTP_TOKEN_STOP (0x08)
+#define T_ESCAPE_LOGITEM (0x10)
int main(int argc, char *argv[])
{
@@ -16,25 +17,27 @@
printf(
"/* this file is automatically generated by gen_test_char, do not edit */\n"
-"#define T_ESCAPE_SHELL_CMD (%u)\n"
-"#define T_ESCAPE_PATH_SEGMENT (%u)\n"
-"#define T_OS_ESCAPE_PATH (%u)\n"
-"#define T_HTTP_TOKEN_STOP (%u)\n"
-"\n"
-"static const unsigned char test_char_table[256] = {\n"
-" 0,",
+"#define T_ESCAPE_SHELL_CMD 0x%02x /* chars with special meaning in the shell */\n"
+"#define T_ESCAPE_PATH_SEGMENT 0x%02x /* find path segment, as defined in RFC1808 */\n"
+"#define T_OS_ESCAPE_PATH 0x%02x /* escape characters in a path or uri */\n"
+"#define T_HTTP_TOKEN_STOP 0x%02x /* find http tokens, as defined in RFC2616 */\n"
+"#define T_ESCAPE_LOGITEM 0x%02x /* filter what should go in the log file */\n"
+"\n",
T_ESCAPE_SHELL_CMD,
T_ESCAPE_PATH_SEGMENT,
T_OS_ESCAPE_PATH,
- T_HTTP_TOKEN_STOP);
+ T_HTTP_TOKEN_STOP,
+ T_ESCAPE_LOGITEM
+ );
/* we explicitly dealt with NUL above
* in case some strchr() do bogosity with it */
+ printf("static const unsigned char test_char_table[256] = {\n"
+ " 0x00, "); /* print initial item */
+
for (c = 1; c < 256; ++c) {
flags = 0;
- if (c % 20 == 0)
- printf("\n ");
/* escape_shell_cmd */
#if defined(WIN32) || defined(OS2)
@@ -67,8 +70,19 @@
if (ap_iscntrl(c) || strchr(" \t()<>@,;:\\/[]?={}", c)) {
flags |= T_HTTP_TOKEN_STOP;
}
- printf("%u%c", flags, (c < 255) ? ',' : ' ');
+ /* For logging, escape all control characters,
+ * double quotes (because they delimit the request in the log file)
+ * backslashes (because we use backslash for escaping)
+ * and 8-bit chars with the high bit set
+ */
+ if (!ap_isprint(c) || c == '"' || c == '\\' || ap_iscntrl(c)) {
+ flags |= T_ESCAPE_LOGITEM;
+ }
+ printf("0x%02x%s", flags, (c < 255) ? ", " : " ");
+
+ if ((c % 8) == 7)
+ printf(" /*0x%02x...0x%02x*/\n ", c-7, c);
}
printf("\n};\n");
1.315 +24 -2 apache-1.3/src/main/http_protocol.c
Index: http_protocol.c
===================================================================
RCS file: /home/cvs/apache-1.3/src/main/http_protocol.c,v
retrieving revision 1.314
retrieving revision 1.315
diff -u -r1.314 -r1.315
--- http_protocol.c 6 Apr 2002 14:31:05 -0000 1.314
+++ http_protocol.c 21 May 2002 13:03:56 -0000 1.315
@@ -983,7 +983,7 @@
const char *uri;
conn_rec *conn = r->connection;
unsigned int major = 1, minor = 0; /* Assume HTTP/1.0 if non-"HTTP" protocol */
- int len;
+ int len, n;
/* Read past empty lines until we get a real request line,
* a read error, the connection closes (EOF), or we timeout.
@@ -1045,12 +1045,26 @@
r->assbackwards = (ll[0] == '\0');
r->protocol = ap_pstrdup(r->pool, ll[0] ? ll : "HTTP/0.9");
- if (2 == sscanf(r->protocol, "HTTP/%u.%u", &major, &minor)
+ if (2 == sscanf(r->protocol, "HTTP/%u.%u%n", &major, &minor, &n)
&& minor < HTTP_VERSION(1,0)) /* don't allow HTTP/0.1000 */
r->proto_num = HTTP_VERSION(major, minor);
else
r->proto_num = HTTP_VERSION(1,0);
+ /* Check for a valid protocol, and disallow everything but whitespace
+ * after the protocol string */
+ while (ap_isspace(r->protocol[n]))
+ ++n;
+ if (r->protocol[n] != '\0') {
+ r->status = HTTP_BAD_REQUEST;
+ r->proto_num = HTTP_VERSION(1,0);
+ r->protocol = ap_pstrdup(r->pool, "HTTP/1.0");
+ ap_table_setn(r->notes, "error-notes",
+ "The request line contained invalid characters "
+ "following the protocol string.<P>\n");
+ return 0;
+ }
+
return 1;
}
@@ -1165,6 +1179,14 @@
ap_log_rerror(APLOG_MARK, APLOG_NOERRNO|APLOG_ERR, r,
"request failed: URI too long");
+ ap_send_error_response(r, 0);
+ ap_log_transaction(r);
+ return r;
+ }
+ else if (r->status == HTTP_BAD_REQUEST) {
+ ap_log_rerror(APLOG_MARK, APLOG_NOERRNO|APLOG_ERR, r,
+ "request failed: erroneous characters after protocol string: %s",
+ ap_escape_logitem(r->pool, r->the_request));
ap_send_error_response(r, 0);
ap_log_transaction(r);
return r;
1.204 +53 -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.203
retrieving revision 1.204
diff -u -r1.203 -r1.204
--- util.c 21 Mar 2002 16:01:31 -0000 1.203
+++ util.c 21 May 2002 13:03:56 -0000 1.204
@@ -1446,6 +1446,59 @@
return (strncasecmp(&line[lidx], tok, tlen) == 0);
}
+
+/* escape a string for logging */
+API_EXPORT(char *) ap_escape_logitem(pool *p, const char *str)
+{
+ char *ret;
+ unsigned char *d;
+ const unsigned char *s;
+
+ if (str == NULL)
+ return NULL;
+
+ ret = ap_palloc(p, 4 * strlen(str) + 1); /* Be safe */
+ d = (unsigned char *)ret;
+ s = (const unsigned char *)str;
+ for (; *s; ++s) {
+
+ if (TEST_CHAR(*s, T_ESCAPE_LOGITEM)) {
+ *d++ = '\\';
+ 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 '\\':
+ case '"':
+ *d++ = *s;
+ break;
+ default:
+ c2x(*s, d);
+ *d = 'x';
+ d += 3;
+ }
+ }
+ else
+ *d++ = *s;
+ }
+ *d = '\0';
+
+ return ret;
+}
+
+
API_EXPORT(char *) ap_escape_shell_cmd(pool *p, const char *str)
{
char *cmd;
1.88 +17 -10 apache-1.3/src/modules/standard/mod_log_config.c
Index: mod_log_config.c
===================================================================
RCS file: /home/cvs/apache-1.3/src/modules/standard/mod_log_config.c,v
retrieving revision 1.87
retrieving revision 1.88
diff -u -r1.87 -r1.88
--- mod_log_config.c 13 Mar 2002 21:05:33 -0000 1.87
+++ mod_log_config.c 21 May 2002 13:03:56 -0000 1.88
@@ -291,8 +291,8 @@
static const char *log_remote_host(request_rec *r, char *a)
{
- return ap_get_remote_host(r->connection, r->per_dir_config,
- REMOTE_NAME);
+ return ap_escape_logitem(r->pool, ap_get_remote_host(r->connection, r->per_dir_config,
+ REMOTE_NAME));
}
static const char *log_remote_address(request_rec *r, char *a)
@@ -307,7 +307,7 @@
static const char *log_remote_logname(request_rec *r, char *a)
{
- return ap_get_remote_logname(r);
+ return ap_escape_logitem(r->pool, ap_get_remote_logname(r));
}
static const char *log_remote_user(request_rec *r, char *a)
@@ -320,6 +320,8 @@
else if (strlen(rvalue) == 0) {
rvalue = "\"\"";
}
+ else
+ rvalue = ap_escape_logitem(r->pool, rvalue);
return rvalue;
}
@@ -330,10 +332,12 @@
* (note the truncation before the protocol string for HTTP/0.9 requests)
* (note also that r->the_request contains the unmodified request)
*/
- return (r->parsed_uri.password) ? ap_pstrcat(r->pool, r->method, " ",
+ return ap_escape_logitem(r->pool,
+ (r->parsed_uri.password) ? ap_pstrcat(r->pool, r->method, " ",
ap_unparse_uri_components(r->pool, &r->parsed_uri, 0),
r->assbackwards ? NULL : " ", r->protocol, NULL)
- : r->the_request;
+ : r->the_request
+ );
}
static const char *log_request_file(request_rec *r, char *a)
@@ -342,19 +346,20 @@
}
static const char *log_request_uri(request_rec *r, char *a)
{
- return r->uri;
+ return ap_escape_logitem(r->pool, r->uri);
}
static const char *log_request_method(request_rec *r, char *a)
{
- return r->method;
+ return ap_escape_logitem(r->pool, r->method);
}
static const char *log_request_protocol(request_rec *r, char *a)
{
- return r->protocol;
+ return ap_escape_logitem(r->pool, r->protocol);
}
static const char *log_request_query(request_rec *r, char *a)
{
- return (r->args != NULL) ? ap_pstrcat(r->pool, "?", r->args, NULL)
+ return (r->args != NULL) ? ap_pstrcat(r->pool, "?",
+ ap_escape_logitem(r->pool, r->args), NULL)
: "";
}
static const char *log_status(request_rec *r, char *a)
@@ -389,7 +394,7 @@
static const char *log_header_in(request_rec *r, char *a)
{
- return ap_table_get(r->headers_in, a);
+ return ap_escape_logitem(r->pool, ap_table_get(r->headers_in, a));
}
static const char *log_header_out(request_rec *r, char *a)
@@ -470,6 +475,7 @@
{
return ap_psprintf(r->pool, "%ld", (long) getpid());
}
+
static const char *log_connection_status(request_rec *r, char *a)
{
if (r->connection->aborted)
@@ -482,6 +488,7 @@
return "-";
}
+
/*****************************************************************
*
* Parsing the log format string
1.39 +1 -0 apache-1.3/src/support/httpd.exp
Index: httpd.exp
===================================================================
RCS file: /home/cvs/apache-1.3/src/support/httpd.exp,v
retrieving revision 1.38
retrieving revision 1.39
diff -u -r1.38 -r1.39
--- httpd.exp 6 Apr 2002 14:31:05 -0000 1.38
+++ httpd.exp 21 May 2002 13:03:56 -0000 1.39
@@ -106,6 +106,7 @@
ap_each_byterange
ap_error_log2stderr
ap_escape_html
+ap_escape_logitem
ap_escape_path_segment
ap_escape_quotes
ap_escape_shell_cmd