topology: harden binary topology decoder against malformed input - #526
Open
HarshRajSinghania wants to merge 1 commit into
Open
HarshRajSinghania wants to merge 1 commit into
HarshRajSinghania wants to merge 1 commit into
Conversation
The binary topology decoder trusts several length/count fields taken from the input .tplg blob. Three related issues are addressed: - decoder.c: snd_tplg_decode() computed hdr->size + hdr->payload_size in 32-bit, which can wrap and bypass the payload bounds check; do the arithmetic in 64-bit and compute the advance in size_t. - dapm.c: tplg_decode_dapm_widget() dereferenced the control header before validating the remaining size; add size checks before reading the ctl/mixer/enum/bytes headers and compute size2 in size_t. - dapm.c: tplg_decode_dapm_graph() passed an attacker-influenced size to alloca(); use calloc()+free() so oversized input fails gracefully. These only affect the (debug-oriented) decode path when processing untrusted or corrupted topology data. Signed-off-by: Harsh Raj Singhania <raj.harshraut@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
As discussed over email with Jaroslav, this hardens the binary topology decoder
against malformed / untrusted
.tplginput. Three related issues, each confirmedwith AddressSanitizer:
snd_tplg_decode()(src/topology/decoder.c):hdr->size + hdr->payload_sizeis evaluated in 32-bit and can wrap, bypassing the payload bounds check; the
block decoder is then called with the raw (huge)
payload_size, causing anout-of-bounds read. Fixed by doing the arithmetic in 64-bit (and the advance
in
size_t).tplg_decode_dapm_widget()(src/topology/dapm.c): the kcontrol loop reads thecontrol header (
chdr->type, thenmc/ec/bc->sizeand->priv.size) beforethe
size2 > sizecheck, giving an out-of-bounds read on a truncated payload.Fixed by adding
size < sizeof(*chdr)/sizeof(*mc|ec|bc)guards andcomputing
size2insize_t.tplg_decode_dapm_graph()(src/topology/dapm.c):alloca()is called with asize derived from the input payload, allowing stack exhaustion on large input.
Fixed by switching to
calloc()/free(), which fails gracefully.Scope is the debug-oriented decode path only, as you noted. Standalone ASan PoC
harnesses are available if useful.
Signed-off-by: Harsh Raj Singhania raj.harshraut@gmail.com