Commit f7fdface authored by Michal Srb's avatar Michal Srb Committed by Derek Foreman

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's avatarPekka Paalanen <pekka.paalanen@collabora.co.uk>
Reviewed-by: Derek Foreman's avatarDerek Foreman <derek.foreman.samsung@gmail.com>
parent f5b9e3b9
Pipeline #3307 passed with stage
in 2 minutes and 44 seconds
...@@ -686,7 +686,7 @@ wl_connection_demarshal(struct wl_connection *connection, ...@@ -686,7 +686,7 @@ wl_connection_demarshal(struct wl_connection *connection,
struct wl_map *objects, struct wl_map *objects,
const struct wl_message *message) const struct wl_message *message)
{ {
uint32_t *p, *next, *end, length, id; uint32_t *p, *next, *end, length, length_in_u32, id;
int fd; int fd;
char *s; char *s;
int i, count, num_arrays; int i, count, num_arrays;
...@@ -742,8 +742,8 @@ wl_connection_demarshal(struct wl_connection *connection, ...@@ -742,8 +742,8 @@ wl_connection_demarshal(struct wl_connection *connection,
break; break;
} }
next = p + div_roundup(length, sizeof *p); length_in_u32 = div_roundup(length, sizeof *p);
if (next > end) { if ((uint32_t) (end - p) < length_in_u32) {
wl_log("message too short, " wl_log("message too short, "
"object (%d), message %s(%s)\n", "object (%d), message %s(%s)\n",
closure->sender_id, message->name, closure->sender_id, message->name,
...@@ -751,6 +751,7 @@ wl_connection_demarshal(struct wl_connection *connection, ...@@ -751,6 +751,7 @@ wl_connection_demarshal(struct wl_connection *connection,
errno = EINVAL; errno = EINVAL;
goto err; goto err;
} }
next = p + length_in_u32;
s = (char *) p; s = (char *) p;
...@@ -801,8 +802,8 @@ wl_connection_demarshal(struct wl_connection *connection, ...@@ -801,8 +802,8 @@ wl_connection_demarshal(struct wl_connection *connection,
case 'a': case 'a':
length = *p++; length = *p++;
next = p + div_roundup(length, sizeof *p); length_in_u32 = div_roundup(length, sizeof *p);
if (next > end) { if ((uint32_t) (end - p) < length_in_u32) {
wl_log("message too short, " wl_log("message too short, "
"object (%d), message %s(%s)\n", "object (%d), message %s(%s)\n",
closure->sender_id, message->name, closure->sender_id, message->name,
...@@ -810,6 +811,7 @@ wl_connection_demarshal(struct wl_connection *connection, ...@@ -810,6 +811,7 @@ wl_connection_demarshal(struct wl_connection *connection,
errno = EINVAL; errno = EINVAL;
goto err; goto err;
} }
next = p + length_in_u32;
array_extra->size = length; array_extra->size = length;
array_extra->alloc = 0; array_extra->alloc = 0;
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment