Commit 5935332a authored by Derek Foreman's avatar Derek Foreman

Merge branch 'mr/request-bad-size' into 'master'

Fix demarshal of invalid size in message header

Closes #52

See merge request !2
parents 808bca89 bace3cd8
Pipeline #23790 passed with stage
in 2 minutes and 41 seconds
......@@ -695,6 +695,14 @@ wl_connection_demarshal(struct wl_connection *connection,
struct wl_closure *closure;
struct wl_array *array_extra;
/* Space for sender_id and opcode */
if (size < 2 * sizeof *p) {
wl_log("message too short, invalid header\n");
wl_connection_consume(connection, size);
errno = EINVAL;
return NULL;
}
closure = wl_closure_init(message, size, &num_arrays, NULL);
if (closure == NULL) {
wl_connection_consume(connection, size);
......
......@@ -752,3 +752,90 @@ TEST(closure_leaks_after_error)
display_destroy(d);
}
/** Raw read from socket expecting wl_display.error
*
* \param sockfd The socket to read from.
* \param expected_error The expected wl_display error code.
*
* Reads the socket and manually parses one message, expecting it to be a
* wl_display.error with the wl_display as the originating object.
* Asserts that the received error code is expected_error.
*/
static void
expect_error_recv(int sockfd, uint32_t expected_error)
{
uint32_t buf[1024];
ssize_t slen;
uint32_t opcode;
int str_len;
slen = recv(sockfd, buf, sizeof buf, 0);
assert(slen >= 2 * (ssize_t)sizeof (uint32_t));
opcode = buf[1] & 0xffff;
fprintf(stderr, "Received %zd bytes, object %u, opcode %u\n",
slen, buf[0], opcode);
/* check error event */
assert(buf[0] == 1);
assert(opcode == WL_DISPLAY_ERROR);
str_len = buf[4];
assert(str_len > 0);
assert(str_len <= slen - 5 * (ssize_t)sizeof (uint32_t));
fprintf(stderr, "Error event on object %u, code %u, message \"%*s\"\n",
buf[2], buf[3], str_len, (const char *)&buf[5]);
assert(buf[3] == expected_error);
}
/* A test for https://gitlab.freedesktop.org/wayland/wayland/issues/52
* trying to provoke a read from uninitialized memory in
* wl_connection_demarshal() for sender_id and opcode.
*
* This test might not fail as is even with #52 unfixed, since there is no way
* to detect what happens and the crash with zero size depends on stack content.
* However, running under Valgrind would point out invalid reads and use of
* uninitialized values.
*/
TEST(request_bogus_size)
{
struct wl_display *display;
struct wl_client *client;
int s[2];
uint32_t msg[3];
int bogus_size;
test_set_timeout(1);
/*
* The manufactured message has real size 12. Test all bogus sizes
* smaller than that, and zero as the last one since wl_closure_init
* handles zero specially and having garbage in the stack makes it more
* likely to crash in wl_connection_demarshal.
*/
for (bogus_size = 11; bogus_size >= 0; bogus_size--) {
fprintf(stderr, "* bogus size %d\n", bogus_size);
assert(socketpair(AF_UNIX, SOCK_STREAM | SOCK_CLOEXEC, 0, s) == 0);
display = wl_display_create();
assert(display);
client = wl_client_create(display, s[0]);
assert(client);
/* manufacture a request that lies about its size */
msg[0] = 1; /* sender id: wl_display */
msg[1] = (bogus_size << 16) | WL_DISPLAY_SYNC; /* size and opcode */
msg[2] = 2; /* sync argument: new_id for wl_callback */
assert(send(s[1], msg, sizeof msg, 0) == sizeof msg);
wl_event_loop_dispatch(wl_display_get_event_loop(display), 0);
expect_error_recv(s[1], WL_DISPLAY_ERROR_INVALID_METHOD);
/* Do not wl_client_destroy, the error already caused it. */
close(s[1]);
wl_display_destroy(display);
}
}
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