dev-libs/wayland: Upstream patch for handling null strings
This fixes an input validation bug where libwayland would accept null
strings as message arguments even when the protocol definition
specified that the string must not be null. The message callback would
likely not expect to receive a null pointer, and non-nullable strings
are the default in wayland protocols, so this allows for both clients
and compositors to trivially trigger crashes in each other.
A quick search shows many instances in both exo and sommelier of
callbacks that don't check for null strings before using them. This
patch fixes all instances where this assumption is justified by the
protocol definition.
I am leaving this in portage-stable for now in the expectation that
this will be accepted in to 1.20.
Upstream merge request:
https://gitlab.freedesktop.org/wayland/wayland/-/merge_requests/149
BUG=none
TEST=CQ
Change-Id: I305186c58202509dd60baa3f44b0763a4bf1e79b
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/overlays/portage-stable/+/2984363
Commit-Queue: Fergus Dall <sidereal@google.com>
Commit-Queue: David Munro <davidmunro@google.com>
Tested-by: Fergus Dall <sidereal@google.com>
Auto-Submit: Fergus Dall <sidereal@google.com>
Reviewed-by: David Munro <davidmunro@google.com>
diff --git a/dev-libs/wayland/files/0001-connection-test-Encode-size-in-message-headers-corre.patch b/dev-libs/wayland/files/0001-connection-test-Encode-size-in-message-headers-corre.patch
new file mode 100644
index 0000000..72e1ca3
--- /dev/null
+++ b/dev-libs/wayland/files/0001-connection-test-Encode-size-in-message-headers-corre.patch
@@ -0,0 +1,78 @@
+From aee23e155474cb3001a8224b247cb6ded7e1f4a3 Mon Sep 17 00:00:00 2001
+From: Fergus Dall <sidereal@google.com>
+Date: Tue, 22 Jun 2021 20:05:47 +1000
+Subject: [PATCH 1/2] connection-test: Encode size in message headers correctly
+
+In these tests, message sizes are inconsistently encoded in either the upper
+or lower 16 bits of the second word of the message. Resolve this in favour
+of using the upper 16 bits, as this is how messages are supposed to be
+encoded, even though that aspect of message decoding isn't being tested
+here.
+
+Signed-off-by: Fergus Dall <sidereal@google.com>
+---
+ tests/connection-test.c | 14 +++++++-------
+ 1 file changed, 7 insertions(+), 7 deletions(-)
+
+diff --git a/tests/connection-test.c b/tests/connection-test.c
+index c04845b..669d73b 100644
+--- a/tests/connection-test.c
++++ b/tests/connection-test.c
+@@ -394,7 +394,7 @@ demarshal(struct marshal_data *data, const char *format,
+ struct wl_closure *closure;
+ struct wl_map objects;
+ struct wl_object object = { NULL, &func, 0 };
+- int size = msg[1];
++ int size = msg[1] >> 16;
+
+ assert(write(data->s[1], msg, size) == size);
+ assert(wl_connection_read(data->read_connection) == size);
+@@ -417,39 +417,39 @@ TEST(connection_demarshal)
+
+ data.value.u = 8000;
+ msg[0] = 400200; /* object id */
+- msg[1] = 12; /* size = 12, opcode = 0 */
++ msg[1] = 12 << 16; /* size = 12, opcode = 0 */
+ msg[2] = data.value.u;
+ demarshal(&data, "u", msg, (void *) validate_demarshal_u);
+
+ data.value.i = -557799;
+ msg[0] = 400200;
+- msg[1] = 12;
++ msg[1] = 12 << 16;
+ msg[2] = data.value.i;
+ demarshal(&data, "i", msg, (void *) validate_demarshal_i);
+
+ data.value.s = "superdude";
+ msg[0] = 400200;
+- msg[1] = 24;
++ msg[1] = 24 << 16;
+ msg[2] = 10;
+ memcpy(&msg[3], data.value.s, msg[2]);
+ demarshal(&data, "s", msg, (void *) validate_demarshal_s);
+
+ data.value.s = "superdude";
+ msg[0] = 400200;
+- msg[1] = 24;
++ msg[1] = 24 << 16;
+ msg[2] = 10;
+ memcpy(&msg[3], data.value.s, msg[2]);
+ demarshal(&data, "?s", msg, (void *) validate_demarshal_s);
+
+ data.value.i = wl_fixed_from_double(-90000.2390);
+ msg[0] = 400200;
+- msg[1] = 12;
++ msg[1] = 12 << 16;
+ msg[2] = data.value.i;
+ demarshal(&data, "f", msg, (void *) validate_demarshal_f);
+
+ data.value.s = NULL;
+ msg[0] = 400200;
+- msg[1] = 12;
++ msg[1] = 12 << 16;
+ msg[2] = 0;
+ demarshal(&data, "?s", msg, (void *) validate_demarshal_s);
+
+--
+2.32.0.93.g670b81a890-goog
+
diff --git a/dev-libs/wayland/files/0002-connection-Handle-non-nullable-strings-in-wl_connect.patch b/dev-libs/wayland/files/0002-connection-Handle-non-nullable-strings-in-wl_connect.patch
new file mode 100644
index 0000000..ad27201
--- /dev/null
+++ b/dev-libs/wayland/files/0002-connection-Handle-non-nullable-strings-in-wl_connect.patch
@@ -0,0 +1,66 @@
+From d514bc6d906ccbb3a8bd9afd2fc214f010c293c6 Mon Sep 17 00:00:00 2001
+From: Fergus Dall <sidereal@google.com>
+Date: Tue, 22 Jun 2021 19:31:26 +1000
+Subject: [PATCH 2/2] connection: Handle non-nullable strings in
+ wl_connection_demarshal
+
+Currently a null string passed into a non-nullable argument of a message
+will decode succesfully, probably resulting in the handler function
+crashing. Instead treat it the same way we do non-nullable objects and ids.
+
+Signed-off-by: Fergus Dall <sidereal@google.com>
+---
+ src/connection.c | 7 +++++++
+ tests/connection-test.c | 18 ++++++++++++++++++
+ 2 files changed, 25 insertions(+)
+
+diff --git a/src/connection.c b/src/connection.c
+index d0c7d9f..557c611 100644
+--- a/src/connection.c
++++ b/src/connection.c
+@@ -749,6 +749,13 @@ wl_connection_demarshal(struct wl_connection *connection,
+ case 's':
+ length = *p++;
+
++ if (length == 0 && !arg.nullable) {
++ wl_log("NULL string received on non-nullable "
++ "type, message %s(%s)\n", message->name,
++ message->signature);
++ errno = EINVAL;
++ goto err;
++ }
+ if (length == 0) {
+ closure->args[i].s = NULL;
+ break;
+diff --git a/tests/connection-test.c b/tests/connection-test.c
+index 669d73b..7220d87 100644
+--- a/tests/connection-test.c
++++ b/tests/connection-test.c
+@@ -553,6 +553,24 @@ expected_fail_demarshal(struct marshal_data *data, const char *format,
+ assert(errno == expected_error);
+ }
+
++TEST(connection_demarshal_null_strings)
++{
++ struct marshal_data data;
++ uint32_t msg[3];
++
++ setup_marshal_data(&data);
++
++ data.value.s = NULL;
++ msg[0] = 400200; /* object id */
++ msg[1] = 12 << 16; /* size = 12, opcode = 0 */
++ msg[2] = 0; /* string length = 0 */
++ demarshal(&data, "?s", msg, (void *) validate_demarshal_s);
++
++ expected_fail_demarshal(&data, "s", msg, EINVAL);
++
++ release_marshal_data(&data);
++}
++
+ /* These tests are verifying that the demarshaling code will gracefully handle
+ * clients lying about string and array lengths and giving values near
+ * UINT32_MAX. Before fixes f7fdface and f5b9e3b9 this test would crash on
+--
+2.32.0.93.g670b81a890-goog
+
diff --git a/dev-libs/wayland/wayland-1.19.0.ebuild b/dev-libs/wayland/wayland-1.19.0-r1.ebuild
similarity index 88%
rename from dev-libs/wayland/wayland-1.19.0.ebuild
rename to dev-libs/wayland/wayland-1.19.0-r1.ebuild
index d1065ab..ef33ea0 100644
--- a/dev-libs/wayland/wayland-1.19.0.ebuild
+++ b/dev-libs/wayland/wayland-1.19.0-r1.ebuild
@@ -35,6 +35,11 @@
"
RDEPEND="${DEPEND}"
+PATCHES=(
+ "${FILESDIR}"/0001-connection-test-Encode-size-in-message-headers-corre.patch
+ "${FILESDIR}"/0002-connection-Handle-non-nullable-strings-in-wl_connect.patch
+)
+
multilib_src_configure() {
local emesonargs=(
$(meson_native_use_bool doc documentation)
