----------------------------------------------------------------------
r1993:  darix | 2007-09-08 01:24:28 +0200
Changed paths:
  M  /branches/lighttpd-1.4.x/src/mod_fastcgi.c

- handle errors from the fcgi packet construction and return 400 if we have an error.

----------------------------------------------------------------------
r1984:  jan | 2007-09-05 13:28:35 +0200
Changed paths:
  M  /branches/lighttpd-1.4.x/src/mod_fastcgi.c

added some extra protection to make sure key-len and val-len don't
overrun

----------------------------------------------------------------------
r1983:  jan | 2007-09-05 13:22:32 +0200
Changed paths:
  M  /branches/lighttpd-1.4.x/NEWS
  M  /branches/lighttpd-1.4.x/src/mod_fastcgi.c

fixed FastCGI header overrun in mod_fastcgi 
  (reported by mattias@secweb.se)

----------------------------------------------------------------------
Index: mod_fastcgi.c
==================================================================
--- mod_fastcgi.c	(/tags/lighttpd-1.4.17/src/mod_fastcgi.c)	(revision 1996)
+++ mod_fastcgi.c	(/branches/lighttpd-1.4.x/src/mod_fastcgi.c)	(revision 1996)
@@ -49,6 +49,12 @@
 #include <sys/wait.h>
 #endif
 
+#define FCGI_ENV_ADD_CHECK(ret, con) \
+	if (ret == -1) { \
+		con->http_status = 400; \
+		con->file_finished = 1; \
+		return -1; \
+	};
 
 /*
  *
@@ -1571,6 +1577,21 @@
 	len += key_len > 127 ? 4 : 1;
 	len += val_len > 127 ? 4 : 1;
 
+	if (env->used + len >= FCGI_MAX_LENGTH) {
+		/**
+		 * we can't append more headers, ignore it
+		 */
+		return -1;
+	}
+
+	/**
+	 * field length can be 31bit max
+	 *
+	 * HINT: this can't happen as FCGI_MAX_LENGTH is only 16bit
+	 */
+	if (key_len > 0x7fffffff) key_len = 0x7fffffff;
+	if (val_len > 0x7fffffff) val_len = 0x7fffffff;
+
 	buffer_prepare_append(env, len);
 
 	if (key_len > 127) {
@@ -1600,6 +1621,8 @@
 }
 
 static int fcgi_header(FCGI_Header * header, unsigned char type, size_t request_id, int contentLength, unsigned char paddingLength) {
+	assert(contentLength <= FCGI_MAX_LENGTH);
+	
 	header->version = FCGI_VERSION_1;
 	header->type = type;
 	header->requestIdB0 = request_id & 0xff;
@@ -1754,7 +1777,7 @@
 			}
 			srv->tmp_buf->ptr[srv->tmp_buf->used++] = '\0';
 
-			fcgi_env_add(p->fcgi_env, CONST_BUF_LEN(srv->tmp_buf), CONST_BUF_LEN(ds->value));
+			FCGI_ENV_ADD_CHECK(fcgi_env_add(p->fcgi_env, CONST_BUF_LEN(srv->tmp_buf), CONST_BUF_LEN(ds->value)),con);
 		}
 	}
 
@@ -1781,7 +1804,7 @@
 			}
 			srv->tmp_buf->ptr[srv->tmp_buf->used++] = '\0';
 
-			fcgi_env_add(p->fcgi_env, CONST_BUF_LEN(srv->tmp_buf), CONST_BUF_LEN(ds->value));
+			FCGI_ENV_ADD_CHECK(fcgi_env_add(p->fcgi_env, CONST_BUF_LEN(srv->tmp_buf), CONST_BUF_LEN(ds->value)), con);
 		}
 	}
 
@@ -1825,10 +1848,10 @@
 	buffer_prepare_copy(p->fcgi_env, 1024);
 
 
-	fcgi_env_add(p->fcgi_env, CONST_STR_LEN("SERVER_SOFTWARE"), CONST_STR_LEN(PACKAGE_NAME"/"PACKAGE_VERSION));
+	FCGI_ENV_ADD_CHECK(fcgi_env_add(p->fcgi_env, CONST_STR_LEN("SERVER_SOFTWARE"), CONST_STR_LEN(PACKAGE_NAME"/"PACKAGE_VERSION)),con)
 
 	if (con->server_name->used) {
-		fcgi_env_add(p->fcgi_env, CONST_STR_LEN("SERVER_NAME"), CONST_BUF_LEN(con->server_name));
+		FCGI_ENV_ADD_CHECK(fcgi_env_add(p->fcgi_env, CONST_STR_LEN("SERVER_NAME"), CONST_BUF_LEN(con->server_name)),con)
 	} else {
 #ifdef HAVE_IPV6
 		s = inet_ntop(srv_sock->addr.plain.sa_family,
@@ -1839,10 +1862,10 @@
 #else
 		s = inet_ntoa(srv_sock->addr.ipv4.sin_addr);
 #endif
-		fcgi_env_add(p->fcgi_env, CONST_STR_LEN("SERVER_NAME"), s, strlen(s));
+		FCGI_ENV_ADD_CHECK(fcgi_env_add(p->fcgi_env, CONST_STR_LEN("SERVER_NAME"), s, strlen(s)),con)
 	}
 
-	fcgi_env_add(p->fcgi_env, CONST_STR_LEN("GATEWAY_INTERFACE"), CONST_STR_LEN("CGI/1.1"));
+	FCGI_ENV_ADD_CHECK(fcgi_env_add(p->fcgi_env, CONST_STR_LEN("GATEWAY_INTERFACE"), CONST_STR_LEN("CGI/1.1")),con)
 
 	LI_ltostr(buf,
 #ifdef HAVE_IPV6
@@ -1852,7 +1875,7 @@
 #endif
 	       );
 
-	fcgi_env_add(p->fcgi_env, CONST_STR_LEN("SERVER_PORT"), buf, strlen(buf));
+	FCGI_ENV_ADD_CHECK(fcgi_env_add(p->fcgi_env, CONST_STR_LEN("SERVER_PORT"), buf, strlen(buf)),con)
 
 	/* get the server-side of the connection to the client */
 	our_addr_len = sizeof(our_addr);
@@ -1862,7 +1885,7 @@
 	} else {
 		s = inet_ntop_cache_get_ip(srv, &(our_addr));
 	}
-	fcgi_env_add(p->fcgi_env, CONST_STR_LEN("SERVER_ADDR"), s, strlen(s));
+	FCGI_ENV_ADD_CHECK(fcgi_env_add(p->fcgi_env, CONST_STR_LEN("SERVER_ADDR"), s, strlen(s)),con)
 
 	LI_ltostr(buf,
 #ifdef HAVE_IPV6
@@ -1872,10 +1895,10 @@
 #endif
 	       );
 
-	fcgi_env_add(p->fcgi_env, CONST_STR_LEN("REMOTE_PORT"), buf, strlen(buf));
+	FCGI_ENV_ADD_CHECK(fcgi_env_add(p->fcgi_env, CONST_STR_LEN("REMOTE_PORT"), buf, strlen(buf)),con)
 
 	s = inet_ntop_cache_get_ip(srv, &(con->dst_addr));
-	fcgi_env_add(p->fcgi_env, CONST_STR_LEN("REMOTE_ADDR"), s, strlen(s));
+	FCGI_ENV_ADD_CHECK(fcgi_env_add(p->fcgi_env, CONST_STR_LEN("REMOTE_ADDR"), s, strlen(s)),con)
 
 	if (!buffer_is_empty(con->authed_user)) {
 		/* AUTH_TYPE fix by Troy Kruthoff (tkruthoff@gmail.com)
@@ -1891,7 +1914,7 @@
 		char *http_authorization = NULL;
 		data_string *ds;
 	  	
-		fcgi_env_add(p->fcgi_env, CONST_STR_LEN("REMOTE_USER"), CONST_BUF_LEN(con->authed_user));
+		FCGI_ENV_ADD_CHECK(fcgi_env_add(p->fcgi_env, CONST_STR_LEN("REMOTE_USER"), CONST_BUF_LEN(con->authed_user)),con)
 	
 		if (NULL != (ds = (data_string *)array_get_element(con->request.headers, "Authorization"))) {
 			http_authorization = ds->value->ptr;
@@ -1915,7 +1938,7 @@
 
 		/* request.content_length < SSIZE_MAX, see request.c */
 		LI_ltostr(buf, con->request.content_length);
-		fcgi_env_add(p->fcgi_env, CONST_STR_LEN("CONTENT_LENGTH"), buf, strlen(buf));
+		FCGI_ENV_ADD_CHECK(fcgi_env_add(p->fcgi_env, CONST_STR_LEN("CONTENT_LENGTH"), buf, strlen(buf)),con)
 	}
 
 	if (host->mode != FCGI_AUTHORIZER) {
@@ -1926,10 +1949,10 @@
 		 * For AUTHORIZER mode these headers should be omitted.
 		 */
 
-		fcgi_env_add(p->fcgi_env, CONST_STR_LEN("SCRIPT_NAME"), CONST_BUF_LEN(con->uri.path));
+		FCGI_ENV_ADD_CHECK(fcgi_env_add(p->fcgi_env, CONST_STR_LEN("SCRIPT_NAME"), CONST_BUF_LEN(con->uri.path)),con)
 
 		if (!buffer_is_empty(con->request.pathinfo)) {
-			fcgi_env_add(p->fcgi_env, CONST_STR_LEN("PATH_INFO"), CONST_BUF_LEN(con->request.pathinfo));
+			FCGI_ENV_ADD_CHECK(fcgi_env_add(p->fcgi_env, CONST_STR_LEN("PATH_INFO"), CONST_BUF_LEN(con->request.pathinfo)),con)
 
 			/* PATH_TRANSLATED is only defined if PATH_INFO is set */
 
@@ -1939,9 +1962,9 @@
 				buffer_copy_string_buffer(p->path, con->physical.doc_root);
 			}
 			buffer_append_string_buffer(p->path, con->request.pathinfo);
-			fcgi_env_add(p->fcgi_env, CONST_STR_LEN("PATH_TRANSLATED"), CONST_BUF_LEN(p->path));
+			FCGI_ENV_ADD_CHECK(fcgi_env_add(p->fcgi_env, CONST_STR_LEN("PATH_TRANSLATED"), CONST_BUF_LEN(p->path)),con)
 		} else {
-			fcgi_env_add(p->fcgi_env, CONST_STR_LEN("PATH_INFO"), CONST_STR_LEN(""));
+			FCGI_ENV_ADD_CHECK(fcgi_env_add(p->fcgi_env, CONST_STR_LEN("PATH_INFO"), CONST_STR_LEN("")),con)
 		}
 	}
 
@@ -1962,8 +1985,8 @@
 		buffer_copy_string_buffer(p->path, host->docroot);
 		buffer_append_string_buffer(p->path, con->uri.path);
 
-		fcgi_env_add(p->fcgi_env, CONST_STR_LEN("SCRIPT_FILENAME"), CONST_BUF_LEN(p->path));
-		fcgi_env_add(p->fcgi_env, CONST_STR_LEN("DOCUMENT_ROOT"), CONST_BUF_LEN(host->docroot));
+		FCGI_ENV_ADD_CHECK(fcgi_env_add(p->fcgi_env, CONST_STR_LEN("SCRIPT_FILENAME"), CONST_BUF_LEN(p->path)),con)
+		FCGI_ENV_ADD_CHECK(fcgi_env_add(p->fcgi_env, CONST_STR_LEN("DOCUMENT_ROOT"), CONST_BUF_LEN(host->docroot)),con)
 	} else {
 		buffer_copy_string_buffer(p->path, con->physical.path);
 
@@ -1975,8 +1998,8 @@
 			buffer_append_string_buffer(p->path, con->request.pathinfo);
 		}
 
-		fcgi_env_add(p->fcgi_env, CONST_STR_LEN("SCRIPT_FILENAME"), CONST_BUF_LEN(p->path));
-		fcgi_env_add(p->fcgi_env, CONST_STR_LEN("DOCUMENT_ROOT"), CONST_BUF_LEN(con->physical.doc_root));
+		FCGI_ENV_ADD_CHECK(fcgi_env_add(p->fcgi_env, CONST_STR_LEN("SCRIPT_FILENAME"), CONST_BUF_LEN(p->path)),con)
+		FCGI_ENV_ADD_CHECK(fcgi_env_add(p->fcgi_env, CONST_STR_LEN("DOCUMENT_ROOT"), CONST_BUF_LEN(con->physical.doc_root)),con)
 	}
 
 	if (host->strip_request_uri->used > 1) {
@@ -2002,34 +2025,34 @@
 					con->request.orig_uri->ptr + (host->strip_request_uri->used - 2),
 					con->request.orig_uri->used - (host->strip_request_uri->used - 2));
 		} else {
-			fcgi_env_add(p->fcgi_env, CONST_STR_LEN("REQUEST_URI"), CONST_BUF_LEN(con->request.orig_uri));
+			FCGI_ENV_ADD_CHECK(fcgi_env_add(p->fcgi_env, CONST_STR_LEN("REQUEST_URI"), CONST_BUF_LEN(con->request.orig_uri)),con)
 		}
 	} else {
-		fcgi_env_add(p->fcgi_env, CONST_STR_LEN("REQUEST_URI"), CONST_BUF_LEN(con->request.orig_uri));
+		FCGI_ENV_ADD_CHECK(fcgi_env_add(p->fcgi_env, CONST_STR_LEN("REQUEST_URI"), CONST_BUF_LEN(con->request.orig_uri)),con)
 	}
 	if (!buffer_is_equal(con->request.uri, con->request.orig_uri)) {
-		fcgi_env_add(p->fcgi_env, CONST_STR_LEN("REDIRECT_URI"), CONST_BUF_LEN(con->request.uri));
+		FCGI_ENV_ADD_CHECK(fcgi_env_add(p->fcgi_env, CONST_STR_LEN("REDIRECT_URI"), CONST_BUF_LEN(con->request.uri)),con)
 	}
 	if (!buffer_is_empty(con->uri.query)) {
-		fcgi_env_add(p->fcgi_env, CONST_STR_LEN("QUERY_STRING"), CONST_BUF_LEN(con->uri.query));
+		FCGI_ENV_ADD_CHECK(fcgi_env_add(p->fcgi_env, CONST_STR_LEN("QUERY_STRING"), CONST_BUF_LEN(con->uri.query)),con)
 	} else {
-		fcgi_env_add(p->fcgi_env, CONST_STR_LEN("QUERY_STRING"), CONST_STR_LEN(""));
+		FCGI_ENV_ADD_CHECK(fcgi_env_add(p->fcgi_env, CONST_STR_LEN("QUERY_STRING"), CONST_STR_LEN("")),con)
 	}
 
 	s = get_http_method_name(con->request.http_method);
-	fcgi_env_add(p->fcgi_env, CONST_STR_LEN("REQUEST_METHOD"), s, strlen(s));
-	fcgi_env_add(p->fcgi_env, CONST_STR_LEN("REDIRECT_STATUS"), CONST_STR_LEN("200")); /* if php is compiled with --force-redirect */
+	FCGI_ENV_ADD_CHECK(fcgi_env_add(p->fcgi_env, CONST_STR_LEN("REQUEST_METHOD"), s, strlen(s)),con)
+	FCGI_ENV_ADD_CHECK(fcgi_env_add(p->fcgi_env, CONST_STR_LEN("REDIRECT_STATUS"), CONST_STR_LEN("200")),con) /* if php is compiled with --force-redirect */
 	s = get_http_version_name(con->request.http_version);
-	fcgi_env_add(p->fcgi_env, CONST_STR_LEN("SERVER_PROTOCOL"), s, strlen(s));
+	FCGI_ENV_ADD_CHECK(fcgi_env_add(p->fcgi_env, CONST_STR_LEN("SERVER_PROTOCOL"), s, strlen(s)),con)
 
 #ifdef USE_OPENSSL
 	if (srv_sock->is_ssl) {
-		fcgi_env_add(p->fcgi_env, CONST_STR_LEN("HTTPS"), CONST_STR_LEN("on"));
+		FCGI_ENV_ADD_CHECK(fcgi_env_add(p->fcgi_env, CONST_STR_LEN("HTTPS"), CONST_STR_LEN("on")),con)
 	}
 #endif
 
 
-	fcgi_env_add_request_headers(srv, con, p);
+	FCGI_ENV_ADD_CHECK(fcgi_env_add_request_headers(srv, con, p), con);
 
 	fcgi_header(&(header), FCGI_PARAMS, request_id, p->fcgi_env->used, 0);
 	buffer_append_memory(b, (const char *)&header, sizeof(header));
@@ -2924,10 +2947,8 @@
 		}
 
 		/* fall through */
-		fcgi_create_env(srv, hctx, hctx->request_id);
-
+		if (-1 == fcgi_create_env(srv, hctx, hctx->request_id)) return HANDLER_ERROR;
 		fcgi_set_state(srv, hctx, FCGI_STATE_WRITE);
-
 		/* fall through */
 	case FCGI_STATE_WRITE:
 		ret = srv->network_backend_write(srv, con, hctx->fd, hctx->wb);
@@ -3109,7 +3130,7 @@
 
 			buffer_reset(con->physical.path);
 			con->mode = DIRECT;
-			con->http_status = 503;
+			if (con->http_status != 400) con->http_status = 503;
 			joblist_append(srv, con); /* really ? */
 
 			return HANDLER_FINISHED;