Index | Thread | Search

From:
Claudio Jeker <cjeker@diehard.n-r-g.com>
Subject:
bgplgd: fix minor issues in slowcgi handler
To:
tech@openbsd.org
Date:
Wed, 20 May 2026 15:37:57 +0200

Download raw body.

Thread
  • Claudio Jeker:

    bgplgd: fix minor issues in slowcgi handler

Fix two issues.

Do not fail with a fatal error when a bad version message is received.
Instead just fail, after logging the fact.
For this switch parse_record to return ssize_t so we can return an error.

Only call exec_cgi once using the command_pid as an indicator.
Now it is possible to fail in exec_cgi before command_pid is set but then
the fastcgi request is also closed. I think this is good enough.

Similar fix should probably be applied to slowcgi.
-- 
:wq Claudio

Index: slowcgi.c
===================================================================
RCS file: /cvs/src/usr.sbin/bgplgd/slowcgi.c,v
diff -u -p -r1.8 slowcgi.c
--- slowcgi.c	16 Dec 2025 15:46:46 -0000	1.8
+++ slowcgi.c	20 May 2026 13:31:13 -0000
@@ -154,7 +154,7 @@ void		slowcgi_response(int, short, void 
 void		slowcgi_add_response(struct request *, struct fcgi_response *);
 void		slowcgi_timeout(int, short, void *);
 void		slowcgi_sig_handler(int, short, void *);
-size_t		parse_record(uint8_t * , size_t, struct request *);
+ssize_t		parse_record(uint8_t * , size_t, struct request *);
 void		parse_begin_request(uint8_t *, uint16_t, struct request *,
 		    uint16_t);
 void		parse_params(uint8_t *, uint16_t, struct request *, uint16_t);
@@ -650,8 +650,7 @@ void
 slowcgi_request(int fd, short events, void *arg)
 {
 	struct request	*c;
-	ssize_t		 n;
-	size_t		 parsed;
+	ssize_t		 n, parsed;
 
 	c = arg;
 
@@ -686,6 +685,8 @@ slowcgi_request(int fd, short events, vo
 	 */
 	do {
 		parsed = parse_record(c->buf + c->buf_pos, c->buf_len, c);
+		if (parsed == -1)
+			goto fail;
 		c->buf_pos += parsed;
 		c->buf_len -= parsed;
 	} while (parsed > 0 && c->buf_len > 0);
@@ -743,6 +744,10 @@ parse_params(uint8_t *buf, uint16_t n, s
 	 * begin execution of the CGI script.
 	 */
 	if (n == 0) {
+		if (c->command_pid != 0) {
+			lwarnx("extra end-of-param, ignoring");
+			return;
+		}
 		exec_cgi(c);
 		return;
 	}
@@ -828,7 +833,7 @@ parse_stdin(uint8_t *buf, uint16_t n, st
 		lwarnx("unexpected stdin input, ignoring");
 }
 
-size_t
+ssize_t
 parse_record(uint8_t *buf, size_t n, struct request *c)
 {
 	struct fcgi_record_header	*h;
@@ -845,8 +850,10 @@ parse_record(uint8_t *buf, size_t n, str
 	    + h->padding_len)
 		return (0);
 
-	if (h->version != 1)
-		lerrx(1, "wrong version");
+	if (h->version != 1) {
+		lwarnx("wrong version");
+		return (-1);
+	}
 
 	switch (h->type) {
 	case FCGI_BEGIN_REQUEST: