commit 0e49c9bbcbfef05d5bdf9ad6d851f51fe25c2eaa
from: Omar Polo <op@omarpolo.com>
date: Fri Jan 21 09:53:47 2022 UTC

gemini_parse_reply: return code and don't close connection

clang static analyzer found a possible use after free: if the code is
not 2X in gemini_parse_reply we call close_conn.  Then, in net_read we
set req->done_header, but req may have been free'd!

Actually, this is almost impossible to trigger.  close_conn never ends
up calling free(req) on its first try, because tls_close takes a while
and is rescheduled by libevent.  (The check req->ctx != NULL in
close_conn is always true if it's a gemini request.)

Nevertheless, it's clear to move close_conn out of gemini_parse_reply
and simply return the response code: it feels wrong that a "parsing
function" takes logic decisions.

commit - 3591e6dbd5cf94dae2b0a438542bf542ae094c01
commit + 0e49c9bbcbfef05d5bdf9ad6d851f51fe25c2eaa
blob - d07b0c84d2a79d85d34267721486841e9e4eda80
blob + 35e3674d618b8b5ad01dd54d45f883e9fde01359
--- net.c
+++ net.c
@@ -510,9 +510,7 @@ gemini_parse_reply(struct req *req, const char *header
 
 	bufferevent_disable(req->bev, EV_READ|EV_WRITE);
 
-	if (code < 20 || code >= 30)
-		close_conn(0, 0, req);
-	return 1;
+	return code;
 }
 
 /* called when we're ready to read/write */
@@ -565,9 +563,11 @@ net_read(struct bufferevent *bev, void *d)
 			return;
 		r = gemini_parse_reply(req, header, len);
 		free(header);
-		if (!r)
+		req->done_header = 1;
+		if (r == 0)
 			goto err;
-		req->done_header = 1;
+		else if (r < 20 || r >= 30)
+			close_conn(0, 0, req);
 		return;
 	}