Skip to content

Commit

Permalink
Avoid a NULL dereference when handling a malformed fastcgi request.
Browse files Browse the repository at this point in the history
Rework the hack to avoid a use-after-free in the fastcgi code.
Since server_fcgi() can be called by server_read_httpcontent() we
can't set clt_fcgi_error to NULL.  Instead, we implement a simple
reference count to track when a fastcgi session is in progress to
avoid closing the http session prematurely on fastcgi error.
Based on a diff from and OK by tb@.  Reported by Ben Kallus.
  • Loading branch information
millert committed Nov 8, 2023
1 parent 4a45226 commit 76ed904
Show file tree
Hide file tree
Showing 3 changed files with 11 additions and 11 deletions.
3 changes: 2 additions & 1 deletion usr.sbin/httpd/httpd.h
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
/* $OpenBSD: httpd.h,v 1.163 2023/07/12 12:37:27 tb Exp $ */
/* $OpenBSD: httpd.h,v 1.164 2023/11/08 19:19:10 millert Exp $ */

/*
* Copyright (c) 2006 - 2015 Reyk Floeter <reyk@openbsd.org>
Expand Down Expand Up @@ -350,6 +350,7 @@ struct client {
int clt_done;
int clt_chunk;
int clt_inflight;
int clt_fcgi_count;
struct range_data clt_ranges;
struct fcgi_data clt_fcgi;
const char *clt_fcgi_error;
Expand Down
4 changes: 2 additions & 2 deletions usr.sbin/httpd/server.c
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
/* $OpenBSD: server.c,v 1.128 2023/09/03 10:18:18 nicm Exp $ */
/* $OpenBSD: server.c,v 1.129 2023/11/08 19:19:10 millert Exp $ */

/*
* Copyright (c) 2006 - 2015 Reyk Floeter <reyk@openbsd.org>
Expand Down Expand Up @@ -1300,7 +1300,7 @@ server_close(struct client *clt, const char *msg)
{
struct server *srv = clt->clt_srv;

if (clt->clt_fcgi_error != NULL) {
if (clt->clt_fcgi_count-- > 0) {
clt->clt_fcgi_error = msg;
return;
}
Expand Down
15 changes: 7 additions & 8 deletions usr.sbin/httpd/server_fcgi.c
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
/* $OpenBSD: server_fcgi.c,v 1.96 2023/07/12 12:37:28 tb Exp $ */
/* $OpenBSD: server_fcgi.c,v 1.97 2023/11/08 19:19:10 millert Exp $ */

/*
* Copyright (c) 2014 Florian Obser <florian@openbsd.org>
Expand Down Expand Up @@ -374,16 +374,15 @@ server_fcgi(struct httpd *env, struct client *clt)
if (clt->clt_toread != 0) {
/*
* XXX - Work around UAF: server_read_httpcontent() can call
* server_close(), normally freeing clt. If clt->clt_fcgi_error
* changed, call server_close() via server_abort_http().
* server_close(), normally freeing clt. If clt->clt_fcgi_count
* reaches 0, call server_close() via server_abort_http().
*/
clt->clt_fcgi_error = "";
clt->clt_fcgi_count++;
server_read_httpcontent(clt->clt_bev, clt);
errstr = clt->clt_fcgi_error;
clt->clt_fcgi_error = NULL;
if (errstr[0] != '\0')
if (clt->clt_fcgi_count-- <= 0) {
errstr = clt->clt_fcgi_error;
goto fail;
errstr = NULL;
}
bufferevent_enable(clt->clt_bev, EV_READ);
} else {
bufferevent_disable(clt->clt_bev, EV_READ);
Expand Down

0 comments on commit 76ed904

Please sign in to comment.