From 87e2b7d6c27de83e63e25d77a82a6d11f703dd41 Mon Sep 17 00:00:00 2001 From: Demi Marie Obenour Date: Wed, 24 Jul 2024 21:17:49 -0400 Subject: [PATCH 1/2] Reject messages not multiple of 4 bytes At least libwayland and ocaml-wayland will never produce such messages, and it is at best ambiguous whether they are allowed by the specification. There are also implementations of the Wayland protocol that reject such messages, so using them has never been portable. Signed-off-by: Demi Marie Obenour --- doc/publican/sources/Protocol.xml | 5 +++-- src/connection.c | 9 ++++++++- src/wayland-server.c | 9 +++++++++ 3 files changed, 20 insertions(+), 3 deletions(-) diff --git a/doc/publican/sources/Protocol.xml b/doc/publican/sources/Protocol.xml index 38243fa7..af5f437d 100644 --- a/doc/publican/sources/Protocol.xml +++ b/doc/publican/sources/Protocol.xml @@ -110,8 +110,9 @@ - The second has 2 parts of 16-bit. The upper 16-bits are the message - size in bytes, starting at the header (i.e. it has a minimum value of 8).The lower is the request/event opcode. + The second has 2 parts of 16 bits each. The upper 16 bits are the message + size in bytes, starting at the header (i.e. it has a minimum value of 8). + The lower is the request/event opcode. The size must be a multiple of 4. diff --git a/src/connection.c b/src/connection.c index e1b751ac..93f5a090 100644 --- a/src/connection.c +++ b/src/connection.c @@ -902,7 +902,14 @@ wl_connection_demarshal(struct wl_connection *connection, /* Space for sender_id and opcode */ if (size < 2 * sizeof *p) { - wl_log("message too short, invalid header\n"); + wl_log("message length %" PRIu32 " too short, invalid header\n", size); + wl_connection_consume(connection, size); + errno = EINVAL; + return NULL; + } + + if (size % sizeof *p) { + wl_log("message length %" PRIu32 " invalid, not multiple of 4 bytes", size); wl_connection_consume(connection, size); errno = EINVAL; return NULL; diff --git a/src/wayland-server.c b/src/wayland-server.c index 1d6be3ec..b7158244 100644 --- a/src/wayland-server.c +++ b/src/wayland-server.c @@ -400,6 +400,15 @@ wl_client_connection_data(int fd, uint32_t mask, void *data) if (len < size) break; + if ((size & 3) != 0) { + /* Post a better error than "invalid arguments" */ + wl_resource_post_error(client->display_resource, + WL_DISPLAY_ERROR_INVALID_METHOD, + "message length %u is not multiple of 4", + size); + break; + } + resource = wl_map_lookup(&client->objects, p[0]); resource_flags = wl_map_lookup_flags(&client->objects, p[0]); if (resource == NULL) { From 0134cf4cdefabb23b0f265167faab237045cd8e0 Mon Sep 17 00:00:00 2001 From: Demi Marie Obenour Date: Wed, 24 Jul 2024 21:21:21 -0400 Subject: [PATCH 2/2] Reject messages with trailing junk These will never be sent by libwayland and likely indicate that a peer is using a wrong protocol specification. Furthermore, there are other implementations that also reject such messages, so using them is not portable. Signed-off-by: Demi Marie Obenour --- doc/publican/sources/Protocol.xml | 2 ++ src/connection.c | 6 ++++++ 2 files changed, 8 insertions(+) diff --git a/doc/publican/sources/Protocol.xml b/doc/publican/sources/Protocol.xml index af5f437d..6fdfa8b2 100644 --- a/doc/publican/sources/Protocol.xml +++ b/doc/publican/sources/Protocol.xml @@ -113,6 +113,8 @@ The second has 2 parts of 16 bits each. The upper 16 bits are the message size in bytes, starting at the header (i.e. it has a minimum value of 8). The lower is the request/event opcode. The size must be a multiple of 4. + Messages must be sent using the fewest bytes possible, so padding after + the end of a message is not permitted. diff --git a/src/connection.c b/src/connection.c index 93f5a090..b24434a5 100644 --- a/src/connection.c +++ b/src/connection.c @@ -1069,6 +1069,12 @@ wl_connection_demarshal(struct wl_connection *connection, } } + if (p != end) { + wl_log("trailing junk\n"); + errno = EINVAL; + goto err; + } + wl_connection_consume(connection, size); return closure;