Open cujomalainey opened 1 month ago
This warrants a wider discussion. Maybe start from these concrete examples and see if there is something systematic and then file for fine-grained items to address. Do you @cujomalainey a stack trace from fuzz to some example case for e.g. comp_dev_get_first_data()? There is a clear assumption at least one buffer is always connected, but if this check can be avoided, this is indeed a problem.
As comp_dev_get_firstdata() is quite recent addition, I'll loop in @marcinszkudlinski to comment on this. Did you consider how to handle NULL returns (it does look you just kept the semantics of the old list_first_item().
I don't have any examples of comp_get_data_blob
yet but its not hard to see the relationship between the binary coming from the IPC (I.e. user space). The issue is not that there is a buffer, its that the buffer is the size we expect. If user space passed in a single byte and we were expecting 20, that is an issue if we don't check the size.
Regarding the buffer checks, here is an example where the fuzzer called trigger on an disconnected selector
==20834==ERROR: AddressSanitizer: SEGV on unknown address 0x00000088 (pc 0x082409de bp 0xec2dfd78 sp 0xec2dfd60 T7) ==20834==The signal is caused by a READ memory access. ==20834==Hint: address points to the zero page. SCARINESS: 10 (null-deref)
#1 0x81fa636 in comp_trigger_local [sof_workspace/sof/src/include/sof/audio/component_ext.h:154](https://github.com/thesofproject/sof/blob/88cde10fecaf778fef4940ee87c83cdc96dcb654/src/include/sof/audio/component_ext.h#L154):8
#2 0x81fa4d8 in comp_trigger [sof_workspace/sof/src/include/sof/audio/component_ext.h:184](https://github.com/thesofproject/sof/blob/88cde10fecaf778fef4940ee87c83cdc96dcb654/src/include/sof/audio/component_ext.h#L184):9
#3 0x81f955f in pipeline_comp_trigger [sof_workspace/sof/src/audio/pipeline/pipeline-stream.c:507](https://github.com/thesofproject/sof/blob/88cde10fecaf778fef4940ee87c83cdc96dcb654/src/audio/pipeline/pipeline-stream.c#L507):8
#4 0x81f7a64 in pipeline_trigger_run [sof_workspace/sof/src/audio/pipeline/pipeline-stream.c:569](https://github.com/thesofproject/sof/blob/88cde10fecaf778fef4940ee87c83cdc96dcb654/src/audio/pipeline/pipeline-stream.c#L569):8
#5 0x81c9fa2 in ipc_stream_trigger [sof_workspace/sof/src/ipc/ipc3/handler.c:522](https://github.com/thesofproject/sof/blob/88cde10fecaf778fef4940ee87c83cdc96dcb654/src/ipc/ipc3/handler.c#L522):9
#6 0x81c48d6 in ipc_cmd [sof_workspace/sof/src/ipc/ipc3/handler.c:1664](https://github.com/thesofproject/sof/blob/88cde10fecaf778fef4940ee87c83cdc96dcb654/src/ipc/ipc3/handler.c#L1664):9
#7 0x81dd8d9 in ipc_platform_do_cmd [sof_workspace/sof/src/platform/posix/ipc.c:160](https://github.com/thesofproject/sof/blob/88cde10fecaf778fef4940ee87c83cdc96dcb654/src/platform/posix/ipc.c#L160):2
#8 0x81d6e0c in ipc_do_cmd [sof_workspace/sof/src/ipc/ipc-common.c:350](https://github.com/thesofproject/sof/blob/88cde10fecaf778fef4940ee87c83cdc96dcb654/src/ipc/ipc-common.c#L350):9
#9 0x81fe70b in task_run [sof_workspace/sof/zephyr/include/rtos/task.h:94](https://github.com/thesofproject/sof/blob/88cde10fecaf778fef4940ee87c83cdc96dcb654/zephyr/include/rtos/task.h#L94):9
#10 0x81fe298 in edf_work_handler [sof_workspace/sof/zephyr/edf_schedule.c:32](https://github.com/thesofproject/sof/blob/88cde10fecaf778fef4940ee87c83cdc96dcb654/zephyr/edf_schedule.c#L32):16
#11 0x8314571 in work_queue_main [sof_workspace/zephyr/kernel/work.c:688](https://github.com/zephyrproject-rtos/zephyr/blob/9d9089edd09919c90c4224222fc2c560410e6c85/kernel/work.c#L688):3
#12 0x81ac4b2 in z_thread_entry [sof_workspace/zephyr/lib/os/thread_entry.c:48](https://github.com/zephyrproject-rtos/zephyr/blob/9d9089edd09919c90c4224222fc2c560410e6c85/lib/os/thread_entry.c#L48):2
Describe the bug Lack of size checks on blobs or topology state. E.g.
comp_dev_get_first_data_*
missing nullity checkscomp_get_data_blob
not checking the size paramsTo Reproduce fuzz
Reproduction Rate high
Expected behavior robust code
Impact security
Possible long term fix Algebraic typing (RUST!)
Short term fixes Some sort of checks on special functions we know are misused using CI tooling