Skip to content

Commit

Permalink
fix an out-of-bound access in start_cgi
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
omar-polo committed Mar 27, 2022
1 parent 6084a9a commit ea27eaa
Show file tree
Hide file tree
Showing 2 changed files with 8 additions and 1 deletion.
1 change: 1 addition & 0 deletions gmid.h
Original file line number Diff line number Diff line change
Expand Up @@ -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];

Expand Down
8 changes: 7 additions & 1 deletion server.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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)) {
Expand Down

0 comments on commit ea27eaa

Please sign in to comment.