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
_______________________________________________
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel
