[PATCH wayland] connection: Don't write past the end of the connection buffer

[Ander Conselvan de Oliveira]

From: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira at intel.com>

If a message was too big to fit in the connection buffer, the code
in wl_buffer_put would just write past the end of it.

I haven't seen any real world use case that would trigger this bug, but
it was possible to trigger it by sending a long enough string to the
wl_data_source.offer request.

https://bugs.freedesktop.org/show_bug.cgi?id=69267
---
 src/connection.c        | 25 +++++++++++++++++--------
 tests/connection-test.c | 37 +++++++++++++++++++++++++++++++++++++
 2 files changed, 54 insertions(+), 8 deletions(-)

diff --git a/src/connection.c b/src/connection.c
index 40a2182..63b0592 100644
--- a/src/connection.c
+++ b/src/connection.c
@@ -61,11 +61,18 @@ struct wl_connection {
 	int want_flush;
 };
 
-static void
+static int
 wl_buffer_put(struct wl_buffer *b, const void *data, size_t count)
 {
 	uint32_t head, size;
 
+	if (count > sizeof(b->data)) {
+		wl_log("Data too big for buffer (%d > %d).\n",
+		       count, sizeof(b->data));
+		errno = E2BIG;
+		return -1;
+	}
+
 	head = MASK(b->head);
 	if (head + count <= sizeof b->data) {
 		memcpy(b->data + head, data, count);
@@ -76,6 +83,8 @@ wl_buffer_put(struct wl_buffer *b, const void *data, size_t count)
 	}
 
 	b->head += count;
+
+	return 0;
 }
 
 static void
@@ -243,8 +252,8 @@ decode_cmsg(struct wl_buffer *buffer, struct msghdr *msg)
 			size /= sizeof(int32_t);
 			for (i = 0; i < size; i++)
 				close(((int*)CMSG_DATA(cmsg))[i]);
-		} else {
-			wl_buffer_put(buffer, CMSG_DATA(cmsg), size);
+		} else if (wl_buffer_put(buffer, CMSG_DATA(cmsg), size) < 0) {
+				return -1;
 		}
 	}
 
@@ -350,7 +359,9 @@ wl_connection_write(struct wl_connection *connection,
 			return -1;
 	}
 
-	wl_buffer_put(&connection->out, data, count);
+	if (wl_buffer_put(&connection->out, data, count) < 0)
+		return -1;
+
 	connection->want_flush = 1;
 
 	return 0;
@@ -367,7 +378,7 @@ wl_connection_queue(struct wl_connection *connection,
 			return -1;
 	}
 
-	wl_buffer_put(&connection->out, data, count);
+	return wl_buffer_put(&connection->out, data, count);
 
 	return 0;
 }
@@ -394,9 +405,7 @@ wl_connection_put_fd(struct wl_connection *connection, int32_t fd)
 			return -1;
 	}
 
-	wl_buffer_put(&connection->fds_out, &fd, sizeof fd);
-
-	return 0;
+	return wl_buffer_put(&connection->fds_out, &fd, sizeof fd);
 }
 
 const char *
diff --git a/tests/connection-test.c b/tests/connection-test.c
index 52d235d..1213875 100644
--- a/tests/connection-test.c
+++ b/tests/connection-test.c
@@ -235,6 +235,27 @@ expected_fail_marshal(int expected_error, const char *format, ...)
 	assert(errno == expected_error);
 }
 
+static void
+expected_fail_marshal_send(struct marshal_data *data, int expected_error,
+			   const char *format, ...)
+{
+	struct wl_closure *closure;
+	static const uint32_t opcode = 4444;
+	static struct wl_object sender = { NULL, NULL, 1234 };
+	struct wl_message message = { "test", format, NULL };
+	va_list ap;
+
+	va_start(ap, format);
+	closure = wl_closure_vmarshal(&sender, opcode, ap, &message);
+	va_end(ap);
+
+	assert(closure);
+	assert(wl_closure_send(closure, data->write_connection) < 0);
+	assert(errno == expected_error);
+
+	wl_closure_destroy(closure);
+}
+
 TEST(connection_marshal_nullables)
 {
 	struct marshal_data data;
@@ -490,6 +511,22 @@ TEST(connection_marshal_alot)
 	release_marshal_data(&data);
 }
 
+TEST(connection_marshal_too_big)
+{
+	struct marshal_data data;
+	char *big_string = malloc(5000);
+
+	memset(big_string, ' ', 4999);
+	big_string[4999] = '\0';
+
+	setup_marshal_data(&data);
+
+	expected_fail_marshal_send(&data, E2BIG, "s", big_string);
+
+	release_marshal_data(&data);
+	free(big_string);
+}
+
 static void
 marshal_helper(const char *format, void *handler, ...)
 {
-- 
1.8.3.2