From edcdb5552d48deb2de937965d62e0fbe5a180376 Mon Sep 17 00:00:00 2001 From: Tobias Stoeckmann Date: Fri, 7 May 2021 20:12:57 +0200 Subject: [PATCH] common: handle invalid IPC messages The size of IPC data is stored in an unsigned 32 bit data type within the IPC message header. In order to terminate the received data with a nul byte, one additional byte is allocated. It is not checked if the transmitted size is 2^32 - 1. Adding one more byte would overflow and lead to 0 byte allocation. On 64 bit systems, the recv call with 2^32 - 1 does not fail instantly but reads data from the server into unallocated memory. Prevent override of unallocated memory by aborting communication. Proof of Concept Python server (use 64 bit address sanitized client): ``` import os import socket os.remove('/tmp/sway-poc.socket') server = socket.socket(socket.AF_UNIX, socket.SOCK_STREAM) server.bind('/tmp/sway-poc.socket') server.listen(1) print('waiting for client') (client, address) = server.accept() client.send(b'\x69\x33\x2D\x69\x70\x63\xFF\xFF\xFF\xFF\x00\x00\x00\x00\xFF\xFF\xFF\xFF\xFF') input('sent reply, press enter') client.close() ``` --- common/ipc-client.c | 20 ++++++++++++++++---- include/ipc-client.h | 4 ++-- sway/main.c | 2 +- swaybar/ipc.c | 8 ++++---- swaymsg/main.c | 2 +- 5 files changed, 24 insertions(+), 12 deletions(-) diff --git a/common/ipc-client.c b/common/ipc-client.c index d30212d25..78dec010f 100644 --- a/common/ipc-client.c +++ b/common/ipc-client.c @@ -1,4 +1,5 @@ #define _POSIX_C_SOURCE 200809L +#include #include #include #include @@ -94,9 +95,15 @@ struct ipc_response *ipc_recv_response(int socketfd) { goto error_1; } - memcpy(&response->size, data + sizeof(ipc_magic), sizeof(uint32_t)); + uint32_t size; + memcpy(&size, data + sizeof(ipc_magic), sizeof(uint32_t)); + response->size = size; memcpy(&response->type, data + sizeof(ipc_magic) + sizeof(uint32_t), sizeof(uint32_t)); + if (response->size >= SSIZE_MAX) { + sway_abort("Unable to receive overly long IPC response"); + } + char *payload = malloc(response->size + 1); if (!payload) { goto error_2; @@ -126,11 +133,16 @@ void free_ipc_response(struct ipc_response *response) { free(response); } -char *ipc_single_command(int socketfd, uint32_t type, const char *payload, uint32_t *len) { +char *ipc_single_command(int socketfd, uint32_t type, const char *payload, size_t *len) { char data[IPC_HEADER_SIZE]; + + if (*len > UINT32_MAX) { + sway_abort("Unable to send overly long IPC payload"); + } + uint32_t size = *len; memcpy(data, ipc_magic, sizeof(ipc_magic)); - memcpy(data + sizeof(ipc_magic), len, sizeof(*len)); - memcpy(data + sizeof(ipc_magic) + sizeof(*len), &type, sizeof(type)); + memcpy(data + sizeof(ipc_magic), &size, sizeof(size)); + memcpy(data + sizeof(ipc_magic) + sizeof(size), &type, sizeof(type)); if (write(socketfd, data, IPC_HEADER_SIZE) == -1) { sway_abort("Unable to send IPC header"); diff --git a/include/ipc-client.h b/include/ipc-client.h index d3895023f..a8919571c 100644 --- a/include/ipc-client.h +++ b/include/ipc-client.h @@ -12,7 +12,7 @@ * encoded payload string. */ struct ipc_response { - uint32_t size; + size_t size; uint32_t type; char *payload; }; @@ -29,7 +29,7 @@ int ipc_open_socket(const char *socket_path); * Issues a single IPC command and returns the buffer. len will be updated with * the length of the buffer returned from sway. */ -char *ipc_single_command(int socketfd, uint32_t type, const char *payload, uint32_t *len); +char *ipc_single_command(int socketfd, uint32_t type, const char *payload, size_t *len); /** * Receives a single IPC response and returns an ipc_response. */ diff --git a/sway/main.c b/sway/main.c index 0611e80bf..e367ed362 100644 --- a/sway/main.c +++ b/sway/main.c @@ -87,7 +87,7 @@ void detect_proprietary(int allow_unsupported_gpu) { void run_as_ipc_client(char *command, char *socket_path) { int socketfd = ipc_open_socket(socket_path); - uint32_t len = strlen(command); + size_t len = strlen(command); char *resp = ipc_single_command(socketfd, IPC_COMMAND, command, &len); printf("%s\n", resp); free(resp); diff --git a/swaybar/ipc.c b/swaybar/ipc.c index a64aa1abf..3800a1dd5 100644 --- a/swaybar/ipc.c +++ b/swaybar/ipc.c @@ -19,7 +19,7 @@ #include "util.h" void ipc_send_workspace_command(struct swaybar *bar, const char *ws) { - uint32_t size = strlen("workspace \"\"") + strlen(ws); + size_t size = strlen("workspace \"\"") + strlen(ws); for (size_t i = 0; i < strlen(ws); ++i) { if (ws[i] == '"' || ws[i] == '\\') { ++size; @@ -341,7 +341,7 @@ bool ipc_get_workspaces(struct swaybar *bar) { free_workspaces(&output->workspaces); output->focused = false; } - uint32_t len = 0; + size_t len = 0; char *res = ipc_single_command(bar->ipc_socketfd, IPC_GET_WORKSPACES, NULL, &len); json_object *results = json_tokener_parse(res); @@ -409,13 +409,13 @@ bool ipc_get_workspaces(struct swaybar *bar) { void ipc_execute_binding(struct swaybar *bar, struct swaybar_binding *bind) { sway_log(SWAY_DEBUG, "Executing binding for button %u (release=%d): `%s`", bind->button, bind->release, bind->command); - uint32_t len = strlen(bind->command); + size_t len = strlen(bind->command); free(ipc_single_command(bar->ipc_socketfd, IPC_COMMAND, bind->command, &len)); } bool ipc_initialize(struct swaybar *bar) { - uint32_t len = strlen(bar->id); + size_t len = strlen(bar->id); char *res = ipc_single_command(bar->ipc_socketfd, IPC_GET_BAR_CONFIG, bar->id, &len); if (!ipc_parse_config(bar->config, res)) { diff --git a/swaymsg/main.c b/swaymsg/main.c index 574d3b759..993e08f06 100644 --- a/swaymsg/main.c +++ b/swaymsg/main.c @@ -476,7 +476,7 @@ int main(int argc, char **argv) { int socketfd = ipc_open_socket(socket_path); struct timeval timeout = {.tv_sec = 3, .tv_usec = 0}; ipc_set_recv_timeout(socketfd, timeout); - uint32_t len = strlen(command); + size_t len = strlen(command); char *resp = ipc_single_command(socketfd, type, command, &len); // pretty print the json