From 135620ab648af5186ba9e7b095ed15ffc75f91ac Mon Sep 17 00:00:00 2001 From: Wim Taymans Date: Thu, 23 Apr 2026 14:41:30 +0200 Subject: [PATCH] security: fix missing malloc NULL checks in echo-cancel Memory Safety: High Three malloc calls for ring buffers (rec_buffer, play_buffer, out_buffer) had no NULL checks. If any allocation fails, the NULL pointers would be passed to memset and ringbuffer operations in reset_buffers(), causing a NULL pointer dereference crash. Additionally, the ring size calculations used uint32_t arithmetic which could overflow with large user-configurable buffer.max_size values. Cast to size_t to perform the multiplication in 64-bit, preventing intermediate overflow. Co-Authored-By: Claude Opus 4.6 --- src/modules/module-echo-cancel.c | 21 +++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/src/modules/module-echo-cancel.c b/src/modules/module-echo-cancel.c index 390563db2..cc118565b 100644 --- a/src/modules/module-echo-cancel.c +++ b/src/modules/module-echo-cancel.c @@ -1179,15 +1179,24 @@ static int setup_streams(struct impl *impl) spa_pod_dynamic_builder_clean(&b); - impl->rec_ringsize = sizeof(float) * impl->max_buffer_size * impl->rec_info.rate / 1000; - impl->play_ringsize = sizeof(float) * ((impl->max_buffer_size * impl->play_info.rate / 1000) + impl->buffer_delay); - impl->out_ringsize = sizeof(float) * impl->max_buffer_size * impl->out_info.rate / 1000; - for (i = 0; i < impl->rec_info.channels; i++) + impl->rec_ringsize = (size_t)sizeof(float) * impl->max_buffer_size * impl->rec_info.rate / 1000; + impl->play_ringsize = (size_t)sizeof(float) * ((size_t)impl->max_buffer_size * impl->play_info.rate / 1000 + impl->buffer_delay); + impl->out_ringsize = (size_t)sizeof(float) * impl->max_buffer_size * impl->out_info.rate / 1000; + for (i = 0; i < impl->rec_info.channels; i++) { impl->rec_buffer[i] = malloc(impl->rec_ringsize); - for (i = 0; i < impl->play_info.channels; i++) + if (impl->rec_buffer[i] == NULL) + return -errno; + } + for (i = 0; i < impl->play_info.channels; i++) { impl->play_buffer[i] = malloc(impl->play_ringsize); - for (i = 0; i < impl->out_info.channels; i++) + if (impl->play_buffer[i] == NULL) + return -errno; + } + for (i = 0; i < impl->out_info.channels; i++) { impl->out_buffer[i] = malloc(impl->out_ringsize); + if (impl->out_buffer[i] == NULL) + return -errno; + } reset_buffers(impl);