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 <pekka.paalanen@collabora.co.uk>
Reviewed-by: Derek Foreman <derek.foreman.samsung@gmail.com>
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 @@
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 @@
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 @@
errno = EINVAL;
goto err;
}
+ next = p + length_in_u32;
s = (char *) p;
@@ -801,8 +802,8 @@
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 @@
errno = EINVAL;
goto err;
}
+ next = p + length_in_u32;
array_extra->size = length;
array_extra->alloc = 0;