From 4a854f96bfba68c7f97dde8317a12adcc187b483 Mon Sep 17 00:00:00 2001
From: InterLinked1 <24227567+InterLinked1@users.noreply.github.com>
Date: Tue, 8 Oct 2024 18:11:19 -0400
Subject: [PATCH] io, modules: Fix memory errors and leaks.

* io_tls: Ensure that the TLS session list is rebuilt immediately
  whenever SSL sessions are removed from the list (and subsequently
  freed). Although the list is locked for removal, in the main
  TLS I/O loop, the list isn't rebuilt during an iteration, only
  at the beginning, so if a session was removed at the beginning
  of the function, prior to the poll call, it was possible for now
  freed sessions to be accessed during the session traversal. To
  prevent this, force a list rebuild immediately in these cases.
  This addresses a wide variety of memory corruption / invalid
  accesses previously caused by this module.
* mod_oauth: Run cleanup code on success, not just failure.
* mod_webmail: Run clenaup code on off-nominal failure case.
---
 io/io_tls.c           | 21 +++++++++++++++++++--
 modules/mod_oauth.c   |  2 ++
 modules/mod_webmail.c |  2 +-
 3 files changed, 22 insertions(+), 3 deletions(-)

diff --git a/io/io_tls.c b/io/io_tls.c
index 1ee3ce7..55478f8 100644
--- a/io/io_tls.c
+++ b/io/io_tls.c
@@ -238,11 +238,20 @@ static void ssl_fd_free(struct ssl_fd *sfd)
 	free(sfd);
 }
 
+static int needcreate = 1; /* Whether the list of TLS sessions is stale and needs to be rebuilt */
+
 static int ssl_unregister_fd(SSL *ssl)
 {
 	struct ssl_fd *sfd;
 
-	sfd = RWLIST_WRLOCK_REMOVE_BY_FIELD(&sslfds, ssl, ssl, entry);
+	RWLIST_WRLOCK(&sslfds);
+	sfd = RWLIST_REMOVE_BY_FIELD(&sslfds, ssl, ssl, entry);
+	if (sfd) {
+		/* Any list of SSL sessions is now stale, since it
+		 * could potentially include the session we just removed. */
+		needcreate = 1;
+	}
+	RWLIST_UNLOCK(&sslfds);
 	if (sfd) {
 		ssl_fd_free(sfd);
 		bbs_alertpipe_write(ssl_alert_pipe); /* Notify I/O thread that we removed a fd, although it'll probably detect this anyways. */
@@ -261,6 +270,7 @@ static void ssl_cleanup_fds(void)
 		ssl_fd_free(sfd);
 		c++;
 	}
+	needcreate = 1;
 	RWLIST_UNLOCK(&sslfds);
 	if (c) {
 		bbs_warning("Forcibly removed %d SSL file descriptor%s\n", c, ESS(c));
@@ -322,7 +332,6 @@ static void *ssl_io_thread(void *unused)
 	 * since we print these out, and nfds_t is signed on some platforms (e.g. Linux)
 	 * and unsigned on others (e.g. FreeBSD), so we can't portably print them. */
 	int i, prevfds = 0, oldnumfds = 0, numfds = 0, numssl = 0;
-	int needcreate = 1;
 	char buf[8192];
 	int pending;
 	int inovertime = 0, overtime = 0;
@@ -425,6 +434,14 @@ static void *ssl_io_thread(void *unused)
 			inovertime = 1;
 		}
 		RWLIST_RDLOCK(&sslfds);
+		/* Now that we've acquired the lock, check if any sessions are stale.
+		 * If so, we need to rebuild the list before iterating again,
+		 * to avoid using sessions that were removed and may now be freed. */
+		if (needcreate) {
+			RWLIST_UNLOCK(&sslfds);
+			bbs_debug(4, "TLS session list has become stale since loop iteration began, rebuilding again...\n");
+			continue;
+		}
 		for (i = 0; res > 0 && i < numfds; i++) {
 			int ores;
 			ssize_t wres;
diff --git a/modules/mod_oauth.c b/modules/mod_oauth.c
index 9e3e2d6..09bc81a 100644
--- a/modules/mod_oauth.c
+++ b/modules/mod_oauth.c
@@ -211,6 +211,8 @@ static int refresh_token(struct oauth_client *client)
 	}
 
 	bbs_verb(4, "Refreshed OAuth token '%s' (good for %ds)\n", client->name, expires);
+	json_decref(json);
+	bbs_curl_free(&c);
 	return 0;
 
 cleanup:
diff --git a/modules/mod_webmail.c b/modules/mod_webmail.c
index e5c3f52..0abb125 100644
--- a/modules/mod_webmail.c
+++ b/modules/mod_webmail.c
@@ -3499,7 +3499,7 @@ static int on_text_message(struct ws_session *ws, void *data, const char *buf, s
 		} else {
 			bbs_warning("Command unknown: %s\n", command);
 		}
-		return res;
+		goto cleanup;
 	}
 
 	if (!strcmp(command, "LIST")) {