From e9e47b8e4fc5a3516874a0ab5604b76009b9b993 Mon Sep 17 00:00:00 2001 From: Nedko Arnaudov Date: Thu, 13 Jun 2024 20:24:03 +0300 Subject: [PATCH] Fix [theoretically possible] memory corruption. Acquiring more than two devices via D-Bus reservation protocol helper code could lead memory corruption. Also using device identifiers longer than 64 will not cause memory corruption anymore. strncpy() is now used instead of strcpy(). In jackdbus PipeWire setups, currently (pipewire-1.1.82), this code should not run at all, as libjackserver.so jack control API implementation is dummy. Also, the presumption in PipeWire setups that WirePlumber will do the device reservation, means that this [jackdbus] device reservation code must not be called at all. In JACK2, traditionally MIDI devices are not handled via reservation protocol, and only one device can be reserved (the audio device used by JACK server). One could argue whether jack2 "slave" devices-drivers should involve reservation protocol too. No less for MIDI devices (via alsa raw, alsa seq or some other MIDI back-end for jack). This commit fixes memory corruption in [JACK] setups where more than one device is reserved. The possibility for buffer with long device identification string, e.g. "Midi716491294619269518659286591827391287391827391827391826419825941256948191981723", is also fixed. So big index numbers are unlikely to be observed in the wild unless in malicious actor scenario. See also commit 56f4d13ecb2b1cd9f8fd5786b769c9788c2b1b7f --- dbus/device_reservation.c | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/dbus/device_reservation.c b/dbus/device_reservation.c index a10e042d..4320c2c0 100644 --- a/dbus/device_reservation.c +++ b/dbus/device_reservation.c @@ -31,9 +31,12 @@ #define DEVICE_MAX 2 +/* string like "Audio2", "Midi5" or "Video3" */ +#define MAX_DEVICE_NAME 63 + typedef struct reserved_audio_device { - char device_name[64]; + char device_name[MAX_DEVICE_NAME+1]; rd_device * reserved_device; } reserved_audio_device; @@ -80,6 +83,18 @@ bool device_reservation_acquire(const char * device_name) assert(gReserveCount < DEVICE_MAX); + if (gReserveCount != 0) { + jack_error("Ignoring reservation for more than one device (acquire)"); + return false; + } + + strncpy(gReservedDevice[gReserveCount].device_name, device_name, MAX_DEVICE_NAME); + if (strcmp(gReservedDevice[gReserveCount].device_name, device_name) != 0) + { + jack_error("Ignoring reservation for device with too long name"); + return false; + } + dbus_error_init(&error); if ((ret= rd_acquire( @@ -96,7 +111,6 @@ bool device_reservation_acquire(const char * device_name) return false; } - strcpy(gReservedDevice[gReserveCount].device_name, device_name); gReserveCount++; jack_info("Acquire audio card %s", device_name); return true;