Commit Diff

Commit:
ea27eaaa83d61792e75858dc624c58fe1fa13dc9
From:
Omar Polo <op@omarpolo.com>
Date:
Message:
fix an out-of-bound access in start_cgi Long time ago, client->req was a static buffer so the memcpy was safe. However, it's been since moved to a dynamically allocated string, so it's very often smaller than sizeof(req.buf) (1024), hence the out of bound access which results in a SIGSEGV very often on OpenBSD thanks to Otto' malloc. The situation with the iri parser, client->req and how the request is forwarded to the other process needs to be improved: this is just a fix to address the issue quickly, a better one would be to restructure the iri parser APIs and rethink how the info is forwarded to the ex process.
Actions:
Patch | Tree
commit - 6084a9a5ba263ddc8cd67f7e03f2ee0481d4ea77
commit + ea27eaaa83d61792e75858dc624c58fe1fa13dc9
blob - 6dd1932f3d033e363dc7f9fc893dcab62f562998
blob + 4572db319c860f88e12c05d9f742776a999b0407
--- gmid.h
+++ gmid.h
@@ -229,6 +229,7 @@ struct client {
 	uint32_t	 id;
 	struct tls	*ctx;
 	char		*req;
+	size_t		 reqlen;
 	struct iri	 iri;
 	char		 domain[DOMAIN_NAME_LEN];
 
blob - a66e4ea3d4ac1b607b67d653e648ad75eab918e5
blob + 19e97667cd9b70e02f5043eba1649557858040a1
--- server.c
+++ server.c
@@ -743,7 +743,7 @@ start_cgi(const char *spath, const char *relpath, stru
 
 	memset(&req, 0, sizeof(req));
 
-	memcpy(req.buf, c->req, sizeof(req.buf));
+	memcpy(req.buf, c->req, c->reqlen);
 
 	req.iri_schema_off = c->iri.schema - c->req;
 	req.iri_host_off = c->iri.host - c->req;
@@ -1022,6 +1022,12 @@ client_read(struct bufferevent *bev, void *d)
 	if (c->req == NULL) {
 		/* not enough data yet. */
 		bufferevent_enable(bev, EVBUFFER_READ);
+		return;
+	}
+	c->reqlen = strlen(c->req);
+	if (c->reqlen > 1024+2) {
+		log_err(c, "URL too long");
+		start_reply(c, BAD_REQUEST, "bad request");
 		return;
 	}