bound control name and value copies in tb_parse_amixer#10947
bound control name and value copies in tb_parse_amixer#10947jmestwa-coder wants to merge 1 commit into
Conversation
|
Can one of the admins verify this patch?
|
There was a problem hiding this comment.
Pull request overview
This PR hardens tb_parse_amixer() in the SOF testbench by rejecting over-length control names and values parsed from control scripts, preventing stack buffer overflows when copying into fixed-size buffers.
Changes:
- Add bounds checks for the parsed control name length before copying into
control_name. - Add bounds checks for the parsed control value length before copying into
control_params.
lyakh
left a comment
There was a problem hiding this comment.
for the future: would be good to at least contextualise such commits by prefixing the commit subject with something like "tools: testbench:"
@jmestwa-coder can you regenerate with a commit message too and update the subject. Thanks ! |
tb_parse_amixer() copies the control name and value parsed from a control-script line into two fixed 128-byte stack buffers (control_name, control_params) via memcpy. The copy length is derived from the quote delimiter pointers with no upper bound: - control_name: len = end_str - name_str - find_len, taken from the cset name="..." quotes and never capped to TB_MAX_CTL_NAME_CHARS - control_params: same unchecked length for the value after the closing quote A script line whose name or value exceeds the buffer overflows the stack. The sibling tb_parse_sofctl() parses the same shape safely with strndup(). Reject over-length fields before each memcpy. Signed-off-by: jmestwa-coder <jmestwa@gmail.com>
535f13a to
e68bb29
Compare
|
done, prefixed the subject with |
kv2019i
left a comment
There was a problem hiding this comment.
ready to merge once CI runs pass
tb_parse_amixer copies the parsed control name and value into fixed 128-byte stack buffers with no length check on the data read from the control script:
Reject over-length fields before each memcpy.