diff options
author | Omar Polo <op@omarpolo.com> | 2022-03-27 12:52:59 +0000 |
---|---|---|
committer | Omar Polo <op@omarpolo.com> | 2022-03-27 12:52:59 +0000 |
commit | ea27eaaa83d61792e75858dc624c58fe1fa13dc9 (patch) | |
tree | 910bdbc26e5b8d2d0bd9fecaa7871c479b50ea3f | |
parent | 6084a9a5ba263ddc8cd67f7e03f2ee0481d4ea77 (diff) |
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.
-rw-r--r-- | gmid.h | 1 | ||||
-rw-r--r-- | server.c | 8 |
2 files changed, 8 insertions, 1 deletions
@@ -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]; @@ -743,7 +743,7 @@ start_cgi(const char *spath, const char *relpath, struct client *c) 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; @@ -1024,6 +1024,12 @@ client_read(struct bufferevent *bev, void *d) 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; + } if (!parse_iri(c->req, &c->iri, &parse_err) || !puny_decode(c->iri.host, decoded, sizeof(decoded), &parse_err)) { |