From e9aab62d65aeeddd3c2ee616d5b50decd0d12ee6 Mon Sep 17 00:00:00 2001
From: InterLinked1 <24227567+InterLinked1@users.noreply.github.com>
Date: Fri, 24 Nov 2023 18:58:44 -0500
Subject: [PATCH] net_telnet: Fix node leak.

For TELNETS, if telnet_handshake fails,
bbs_node_exit was not called so the node
was not automatically cleaned up. The node
would thus stay registered indefinitely
until manually kicked by the sysop.

Node setup for network protocols is also
simplified a bit, and we now keeptrack
of a "lifetime node ID", unique for the lifetime
of the BBS process, to make matching up registered
nodes to previous events in the logs a little
bit easier.
---
 bbs/node.c         | 25 ++++++++++++++-----------
 include/node.h     |  9 ++++++++-
 modules/mod_http.c |  8 ++------
 nets/net_finger.c  |  4 +---
 nets/net_ftp.c     |  4 +---
 nets/net_gopher.c  |  4 +---
 nets/net_imap.c    |  8 ++------
 nets/net_irc.c     |  8 ++------
 nets/net_nntp.c    |  8 ++------
 nets/net_pop3.c    |  8 ++------
 nets/net_sieve.c   |  6 ++----
 nets/net_smtp.c    |  7 ++-----
 nets/net_telnet.c  |  2 ++
 nets/net_ws.c      |  8 ++------
 14 files changed, 43 insertions(+), 66 deletions(-)

diff --git a/bbs/node.c b/bbs/node.c
index d23f6399..de6389e6 100644
--- a/bbs/node.c
+++ b/bbs/node.c
@@ -177,7 +177,7 @@ const char *bbs_name(void)
 	return bbs_name_buf;
 }
 
-static int lifetime_nodes = 0;
+static unsigned int lifetime_nodes = 0;
 
 struct bbs_node *__bbs_node_request(int fd, const char *protname, void *mod)
 {
@@ -269,11 +269,10 @@ struct bbs_node *__bbs_node_request(int fd, const char *protname, void *mod)
 	} else {
 		RWLIST_INSERT_HEAD(&nodes, node, entry); /* This is the first node. */
 	}
-	lifetime_nodes++;
+	node->lifetimeid = lifetime_nodes++;
 	RWLIST_UNLOCK(&nodes);
 
-	bbs_debug(1, "Allocated new node with ID %u\n", node->id);
-
+	bbs_debug(1, "Allocated new node with ID %u (lifetime ID %d)\n", node->id, node->lifetimeid);
 	return node;
 }
 
@@ -686,7 +685,7 @@ static int cli_nodes(struct bbs_cli_args *a)
 	}
 	RWLIST_UNLOCK(&nodes);
 
-	bbs_dprintf(a->fdout, "%d active node%s, %d lifetime node%s\n", c, ESS(c), lifetime_nodes, ESS(lifetime_nodes));
+	bbs_dprintf(a->fdout, "%d active node%s, %u lifetime node%s\n", c, ESS(c), lifetime_nodes, ESS(lifetime_nodes));
 	return 0;
 }
 
@@ -791,7 +790,6 @@ static int node_info(int fd, unsigned int nodenum)
 	char connecttime[29];
 	struct bbs_node *n;
 	char menufull[16];
-	int lwp;
 	time_t now = time(NULL);
 
 	RWLIST_RDLOCK(&nodes);
@@ -826,10 +824,9 @@ static int node_info(int fd, unsigned int nodenum)
 		bbs_dprintf(fd, BBS_FMT_S, title,fallback); \
 	}
 
-	lwp = bbs_pthread_tid(n->thread);
-
 	pthread_mutex_lock(&n->lock);
 	bbs_dprintf(fd, BBS_FMT_D, "#", n->id);
+	bbs_dprintf(fd, BBS_FMT_D, "Lifetime #", n->lifetimeid);
 	bbs_dprintf(fd, BBS_FMT_S, "Protocol", n->protname);
 	bbs_dprintf(fd, BBS_FMT_S, "IP Address", n->ip);
 	bbs_dprintf(fd, BBS_FMT_S, "Connected", connecttime);
@@ -842,9 +839,8 @@ static int node_info(int fd, unsigned int nodenum)
 	bbs_dprintf(fd, BBS_FMT_D, "Node PTY Master FD", n->amaster);
 	bbs_dprintf(fd, BBS_FMT_D, "Node PTY Slave FD", n->slavefd);
 	bbs_dprintf(fd, BBS_FMT_S, "Node PTY Slave Name", n->slavename);
-	if (lwp != -1) {
-		bbs_dprintf(fd, BBS_FMT_D, "Node Thread ID", lwp);
-	}
+	bbs_dprintf(fd, BBS_FMT_D, "Node PTY Thread ID", bbs_pthread_tid(n->ptythread));
+	bbs_dprintf(fd, BBS_FMT_D, "Node Thread ID", bbs_pthread_tid(n->thread));
 	bbs_dprintf(fd, BBS_FMT_S, "User", bbs_username(n->user));
 	if (bbs_user_is_guest(n->user)) {
 		bbs_dprintf(fd, BBS_FMT_S, "Guest Name/Alias",  S_IF(n->user->guestname));
@@ -1403,8 +1399,15 @@ void bbs_node_begin(struct bbs_node *node)
 	bbs_auth("New %s connection to node %d from %s:%u\n", node->protname, node->id, node->ip, node->rport);
 }
 
+void bbs_node_net_begin(struct bbs_node *node)
+{
+	node->thread = pthread_self();
+	bbs_node_begin(node);
+}
+
 void bbs_node_exit(struct bbs_node *node)
 {
+	bbs_debug(3, "Node %d has ended its %s session\n", node->id, node->protname);
 	if (node->active) {
 		/* User quit: unlink and free */
 		bbs_node_unlink(node);
diff --git a/include/node.h b/include/node.h
index 5ee347ba..e035d5e7 100644
--- a/include/node.h
+++ b/include/node.h
@@ -26,6 +26,7 @@ struct pollfd;
 
 struct bbs_node {
 	unsigned int id;			/*!< Node number, 1-indexed for user-friendliness */
+	unsigned int lifetimeid;	/*!< Lifetime node number, 1-indexed */
 	int fd;						/*!< Socket file descriptor */
 	int rfd;					/*!< File descriptor for reading */
 	int wfd;					/*!< File descriptor for writing */
@@ -685,13 +686,19 @@ int bbs_node_wait_key(struct bbs_node *node, int ms);
  */
 void bbs_node_begin(struct bbs_node *node);
 
+/*! \brief Begin handling a node from a network protocol
+ * \note Only intended for network protocols that don't use psuedoterminals
+ */
+void bbs_node_net_begin(struct bbs_node *node);
+
 /*! \brief Stop handling a node
  * \note Not needed if you use bbs_node_handler
+ * \note After calling this function, node will no longer be a valid reference
  */
 void bbs_node_exit(struct bbs_node *node) __attribute__ ((nonnull (1)));
 
 /*!
- * \brief Top-level node handler
+ * \brief Top-level node handler for terminal protocols
  * \param varg BBS node
  */
 void *bbs_node_handler(void *varg);
diff --git a/modules/mod_http.c b/modules/mod_http.c
index 7806e3d1..7681b4d5 100644
--- a/modules/mod_http.c
+++ b/modules/mod_http.c
@@ -2651,13 +2651,9 @@ static void *__http_handler(void *varg)
 {
 	struct bbs_node *node = varg;
 
-	node->thread = pthread_self();
-	bbs_node_begin(node);
-
+	bbs_node_net_begin(node);
 	http_handler(node, !strcmp(node->protname, "HTTPS") ? 1 : 0); /* Actually handle the HTTP/HTTPS client */
-
-	bbs_debug(3, "Node %d has ended its %s session\n", node->id, node->protname);
-	bbs_node_exit(node); /* node is no longer a valid reference */
+	bbs_node_exit(node);
 	return NULL;
 }
 
diff --git a/nets/net_finger.c b/nets/net_finger.c
index 23db4c09..a8985ed8 100644
--- a/nets/net_finger.c
+++ b/nets/net_finger.c
@@ -47,9 +47,7 @@ static void *finger_handler(void *varg)
 	int verbose = 0;
 	ssize_t res;
 
-	/* This thread is running instead of the normal node handler thread */
-	/* Remember, no pseudoterminal is allocated for this node! Can NOT use normal bbs_ I/O functions. */
-	bbs_node_begin(node);
+	bbs_node_net_begin(node);
 
 	/* A RUIP (Remote User Information Program) client has connected to us.
 	 * Note that the GNU finger client doesn't seem to work for me but the Windows one does. Go figure.
diff --git a/nets/net_ftp.c b/nets/net_ftp.c
index f1a553e2..d32d0088 100644
--- a/nets/net_ftp.c
+++ b/nets/net_ftp.c
@@ -245,9 +245,7 @@ static void *ftp_handler(void *varg)
 	struct ftp_session ftpstack, *ftp;
 	SSL *ssl = NULL, *ssl2 = NULL;
 
-	/* This thread is running instead of the normal node handler thread */
-	/* Remember, no pseudoterminal is allocated for this node! Can NOT use normal bbs_ I/O functions. */
-	bbs_node_begin(node);
+	bbs_node_net_begin(node);
 
 	if (bbs_get_local_ip(our_ip, sizeof(our_ip))) { /* Determine it just once, now */
 		goto cleanup;
diff --git a/nets/net_gopher.c b/nets/net_gopher.c
index 17d30293..81249d44 100644
--- a/nets/net_gopher.c
+++ b/nets/net_gopher.c
@@ -68,9 +68,7 @@ static void *gopher_handler(void *varg)
 	struct stat st;
 	ssize_t res;
 
-	/* This thread is running instead of the normal node handler thread */
-	/* Remember, no pseudoterminal is allocated for this node! Can NOT use normal bbs_ I/O functions. */
-	bbs_node_begin(node);
+	bbs_node_net_begin(node);
 
 	/* This is not buffered since there's no pseudoterminal. */
 	res = bbs_poll_read(node->fd, 1000, buf, sizeof(buf) - 1); /* Read the retrieval/selector string from the client */
diff --git a/nets/net_imap.c b/nets/net_imap.c
index 5a255b4a..91b3393b 100644
--- a/nets/net_imap.c
+++ b/nets/net_imap.c
@@ -4691,13 +4691,9 @@ static void *__imap_handler(void *varg)
 {
 	struct bbs_node *node = varg;
 
-	node->thread = pthread_self();
-	bbs_node_begin(node);
-
+	bbs_node_net_begin(node);
 	imap_handler(node, !strcmp(node->protname, "IMAPS"));
-
-	bbs_debug(3, "Node %d has ended its %s session\n", node->id, node->protname);
-	bbs_node_exit(node); /* node is no longer a valid reference */
+	bbs_node_exit(node);
 	return NULL;
 }
 
diff --git a/nets/net_irc.c b/nets/net_irc.c
index c4596912..9b7f9bf6 100644
--- a/nets/net_irc.c
+++ b/nets/net_irc.c
@@ -3452,13 +3452,9 @@ static void *__irc_handler(void *varg)
 {
 	struct bbs_node *node = varg;
 
-	node->thread = pthread_self();
-	bbs_node_begin(node);
-
+	bbs_node_net_begin(node);
 	irc_handler(node, !strcmp(node->protname, "IRCS") ? 1 : 0); /* Actually handle the IRC/IRCS client */
-
-	bbs_debug(3, "Node %d has ended its %s session\n", node->id, node->protname);
-	bbs_node_exit(node); /* node is no longer a valid reference */
+	bbs_node_exit(node);
 	return NULL;
 }
 
diff --git a/nets/net_nntp.c b/nets/net_nntp.c
index 93c81535..3a66749d 100644
--- a/nets/net_nntp.c
+++ b/nets/net_nntp.c
@@ -1303,13 +1303,9 @@ static void *__nntp_handler(void *varg)
 {
 	struct bbs_node *node = varg;
 
-	node->thread = pthread_self();
-	bbs_node_begin(node);
-
+	bbs_node_net_begin(node);
 	nntp_handler(node, !strcmp(node->protname, "NNTPS"), !strcmp(node->protname, "NNSP") ? NNTP_MODE_TRANSIT: NNTP_MODE_READER);
-
-	bbs_debug(3, "Node %d has ended its %s session\n", node->id, node->protname);
-	bbs_node_exit(node); /* node is no longer a valid reference */
+	bbs_node_exit(node);
 	return NULL;
 }
 
diff --git a/nets/net_pop3.c b/nets/net_pop3.c
index ae2d700c..fe8977db 100644
--- a/nets/net_pop3.c
+++ b/nets/net_pop3.c
@@ -784,13 +784,9 @@ static void *__pop3_handler(void *varg)
 {
 	struct bbs_node *node = varg;
 
-	node->thread = pthread_self();
-	bbs_node_begin(node);
-
+	bbs_node_net_begin(node);
 	pop3_handler(node, !strcmp(node->protname, "POP3S"));
-
-	bbs_debug(3, "Node %d has ended its %s session\n", node->id, node->protname);
-	bbs_node_exit(node); /* node is no longer a valid reference */
+	bbs_node_exit(node);
 	return NULL;
 }
 
diff --git a/nets/net_sieve.c b/nets/net_sieve.c
index 59217624..f2645329 100644
--- a/nets/net_sieve.c
+++ b/nets/net_sieve.c
@@ -497,8 +497,7 @@ static void *__sieve_handler(void *varg)
 #endif
 	struct sieve_session sieve;
 
-	node->thread = pthread_self();
-	bbs_node_begin(node);
+	bbs_node_net_begin(node);
 
 	memset(&sieve, 0, sizeof(sieve));
 	sieve.rfd = sieve.wfd = node->fd;
@@ -516,8 +515,7 @@ static void *__sieve_handler(void *varg)
 #endif
 	sieve_cleanup(&sieve);
 
-	bbs_debug(3, "Node %d has ended its %s session\n", node->id, node->protname);
-	bbs_node_exit(node); /* node is no longer a valid reference */
+	bbs_node_exit(node);
 	return NULL;
 }
 
diff --git a/nets/net_smtp.c b/nets/net_smtp.c
index 4b415fd3..7b4237f2 100644
--- a/nets/net_smtp.c
+++ b/nets/net_smtp.c
@@ -2646,14 +2646,11 @@ static void *__smtp_handler(void *varg)
 	struct bbs_node *node = varg;
 	int secure = !strcmp(node->protname, "SMTPS") ? 1 : 0;
 
-	node->thread = pthread_self();
-	bbs_node_begin(node);
+	bbs_node_net_begin(node);
 
 	/* If it's secure, it's for message submission agent, MTAs are never secure by default. */
 	smtp_handler(node, secure || !strcmp(node->protname, "SMTP (MSA)"), secure); /* Actually handle the SMTP/SMTPS/message submission agent client */
-
-	bbs_debug(3, "Node %d has ended its %s session\n", node->id, node->protname);
-	bbs_node_exit(node); /* node is no longer a valid reference */
+	bbs_node_exit(node);
 	return NULL;
 }
 
diff --git a/nets/net_telnet.c b/nets/net_telnet.c
index f1d1f4ba..1466e492 100644
--- a/nets/net_telnet.c
+++ b/nets/net_telnet.c
@@ -227,6 +227,8 @@ static void *telnets_handler(void *varg)
 
 	if (!telnet_handshake(node)) {
 		bbs_node_handler(node); /* Run the normal node handler */
+	} else {
+		bbs_node_exit(node); /* Manual cleanup */
 	}
 
 	ssl_close(ssl);
diff --git a/nets/net_ws.c b/nets/net_ws.c
index 8f19e245..2852626f 100644
--- a/nets/net_ws.c
+++ b/nets/net_ws.c
@@ -1174,13 +1174,9 @@ static void *__ws_handler(void *varg)
 {
 	struct bbs_node *node = varg;
 
-	node->thread = pthread_self();
-	bbs_node_begin(node);
-
+	bbs_node_net_begin(node);
 	ws_direct_handler(node, !strcmp(node->protname, "WSS"));
-
-	bbs_debug(3, "Node %d has ended its %s session\n", node->id, node->protname);
-	bbs_node_exit(node); /* node is no longer a valid reference */
+	bbs_node_exit(node);
 	return NULL;
 }