From ff3bc0842e96bbcd4c0e07dd750960ab89bdfa05 Mon Sep 17 00:00:00 2001 From: Andy Ross Date: Thu, 22 Jun 2023 13:19:20 -0700 Subject: [PATCH] ipc3: Don't trust size values in protocol input The handling for SOF_COMP_NONE was extracting a "size" field from a struct sof_ipc_comp_process command and passing it down into the module_adapter layer where it was being used as the range for a memcpy_s(). But that struct is IPC input data from the host! The size can trivially be wrong and cause an overflow. And needless to say, fuzzing discovered the hole and blew it up. Note that the previous behavior when LIBRARY||UNIT_TEST is left unchanged (fuzz builds as "real" firmware and doesn't use those features). It turns out that this is still in production use, though the details of why the code is different are unclear. Drop a note in to fix this later. Signed-off-by: Andy Ross --- src/ipc/ipc3/helper.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/ipc/ipc3/helper.c b/src/ipc/ipc3/helper.c index 116571e13318..48379c8a8b71 100644 --- a/src/ipc/ipc3/helper.c +++ b/src/ipc/ipc3/helper.c @@ -277,10 +277,14 @@ static void comp_specific_builder(struct sof_ipc_comp *comp, case SOF_COMP_MODULE_ADAPTER: case SOF_COMP_NONE: config->process.type = proc->type; - config->process.size = proc->size; #if CONFIG_LIBRARY || UNIT_TEST + /* FIXME: protocol code paths shouldn't be different for testing */ + config->process.size = proc->size; config->process.data = proc->data + comp->ext_data_length; #else + /* Clamp size, don't trust the protocol-supplied value! */ + config->process.size = MIN(proc->size, + SOF_IPC_MSG_MAX_SIZE - sizeof(*proc)); config->process.data = proc->data; #endif break;