From f7fdface41a9205c12aedf7fe04aba7792402909 Mon Sep 17 00:00:00 2001 From: Michal Srb Date: Tue, 14 Aug 2018 13:07:53 +0200 Subject: [PATCH] connection: Prevent pointer overflow from large lengths. 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. Reviewed-by: Pekka Paalanen Reviewed-by: Derek Foreman --- src/connection.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/src/connection.c b/src/connection.c index cb4b8d5..f965210 100644 --- a/src/connection.c +++ b/src/connection.c @@ -686,7 +686,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; @@ -742,8 +742,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, @@ -751,6 +751,7 @@ wl_connection_demarshal(struct wl_connection *connection, errno = EINVAL; goto err; } + next = p + length_in_u32; s = (char *) p; @@ -801,8 +802,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, @@ -810,6 +811,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.7.4