From: Tobias Brunner <tobias@strongswan.org>
Date: Fri, 18 Jul 2025 10:07:45 +0000 (+0200)
Subject: whitelist: Add error handling to socket reads and fix a memory leak
X-Git-Url: http://git.ipfire.org/gitweb/gitweb.cgi?a=commitdiff_plain;h=85ebf6abd4412d241fd55bc076b60f0a87931540;p=thirdparty%2Fstrongswan.git

whitelist: Add error handling to socket reads and fix a memory leak

This now adds some state (basically a message buffer), but simplifies
error handling as we don't have to handle two potential failure paths
and could avoid some potential issues by still calling the blocking
read_all().

It also fixes a memory leak when clients disconnect.
---

diff --git a/src/libcharon/plugins/whitelist/whitelist_control.c b/src/libcharon/plugins/whitelist/whitelist_control.c
index c8dc7ef8d0..4aec53aeef 100644
--- a/src/libcharon/plugins/whitelist/whitelist_control.c
+++ b/src/libcharon/plugins/whitelist/whitelist_control.c
@@ -92,38 +92,51 @@ static void list(private_whitelist_control_t *this,
 	stream->write_all(stream, &msg, sizeof(msg));
 }
 
+/**
+ * Information about a client connection.
+ */
+typedef struct {
+	private_whitelist_control_t *this;
+	whitelist_msg_t msg;
+	size_t read;
+} whitelist_conn_t;
+
 /**
  * Dispatch a received message
  */
 CALLBACK(on_read, bool,
-	private_whitelist_control_t *this, stream_t *stream)
+	whitelist_conn_t *conn, stream_t *stream)
 {
+	private_whitelist_control_t *this = conn->this;
 	identification_t *id;
-	whitelist_msg_t msg;
-	ssize_t n;
+	ssize_t len;
 
 	while (TRUE)
 	{
-		n = stream->read(stream, &msg, sizeof(msg), FALSE);
-		if (n <= 0)
+		while (conn->read < sizeof(conn->msg))
 		{
-			if (errno == EWOULDBLOCK)
-			{
-				break;
-			}
-			return FALSE;
-		}
-		if (n < sizeof(msg))
-		{
-			if (!stream->read_all(stream, ((char *)&msg) + n, sizeof(msg) - n))
+			len = stream->read(stream, (char*)&conn->msg + conn->read,
+							   sizeof(conn->msg) - conn->read, FALSE);
+			if (len <= 0)
 			{
+				if (errno == EWOULDBLOCK)
+				{
+					return TRUE;
+				}
+				if (len != 0)
+				{
+					DBG1(DBG_CFG, "whitelist socket error: %s", strerror(errno));
+				}
+				stream->destroy(stream);
+				free(conn);
 				return FALSE;
 			}
+			conn->read += len;
 		}
 
-		msg.id[sizeof(msg.id) - 1] = 0;
-		id = identification_create_from_string(msg.id);
-		switch (ntohl(msg.type))
+		conn->msg.id[sizeof(conn->msg.id) - 1] = 0;
+		id = identification_create_from_string(conn->msg.id);
+		switch (ntohl(conn->msg.type))
 		{
 			case WHITELIST_ADD:
 				this->listener->add(this->listener, id);
@@ -148,6 +161,7 @@ CALLBACK(on_read, bool,
 				break;
 		}
 		id->destroy(id);
+		conn->read = 0;
 	}
 
 	return TRUE;
@@ -156,7 +170,12 @@ CALLBACK(on_read, bool,
 CALLBACK(on_accept, bool,
 	private_whitelist_control_t *this, stream_t *stream)
 {
-	stream->on_read(stream, on_read, this);
+	whitelist_conn_t *conn;
+
+	INIT(conn,
+		.this = this,
+	);
+	stream->on_read(stream, on_read, conn);
 	return TRUE;
 }