[PATCH 3/3] connection: Prevent pointer overflow from large lengths.

Michal Srb msrb at suse.com
Tue Aug 14 11:07:53 UTC 2018


If the remote side sends sufficiently large `length` field, it will
overflow the `p` pointer. Technically it is undefined behavior, in
practice it makes `p < end`, so the length check passes. Attempts to
access the data later causes crashes.

This issue manifests only on 32bit systems, but the behavior is
undefined everywhere.
---
 src/connection.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/src/connection.c b/src/connection.c
index 31396bc..a0f10ae 100644
--- a/src/connection.c
+++ b/src/connection.c
@@ -683,7 +683,7 @@ wl_connection_demarshal(struct wl_connection *connection,
 			struct wl_map *objects,
 			const struct wl_message *message)
 {
-	uint32_t *p, *next, *end, length, id;
+	uint32_t *p, *next, *end, length, length_in_u32, id;
 	int fd;
 	char *s;
 	int i, count, num_arrays;
@@ -739,8 +739,8 @@ wl_connection_demarshal(struct wl_connection *connection,
 				break;
 			}
 
-			next = p + div_roundup(length, sizeof *p);
-			if (next > end) {
+			length_in_u32 = div_roundup(length, sizeof *p);
+			if ((uint32_t) (end - p) < length_in_u32) {
 				wl_log("message too short, "
 				       "object (%d), message %s(%s)\n",
 				       closure->sender_id, message->name,
@@ -748,6 +748,7 @@ wl_connection_demarshal(struct wl_connection *connection,
 				errno = EINVAL;
 				goto err;
 			}
+			next = p + length_in_u32;
 
 			s = (char *) p;
 
@@ -798,8 +799,8 @@ wl_connection_demarshal(struct wl_connection *connection,
 		case 'a':
 			length = *p++;
 
-			next = p + div_roundup(length, sizeof *p);
-			if (next > end) {
+			length_in_u32 = div_roundup(length, sizeof *p);
+			if ((uint32_t) (end - p) < length_in_u32) {
 				wl_log("message too short, "
 				       "object (%d), message %s(%s)\n",
 				       closure->sender_id, message->name,
@@ -807,6 +808,7 @@ wl_connection_demarshal(struct wl_connection *connection,
 				errno = EINVAL;
 				goto err;
 			}
+			next = p + length_in_u32;
 
 			array_extra->size = length;
 			array_extra->alloc = 0;
-- 
2.16.4



More information about the wayland-devel mailing list