Validate num_channels against tensor sizes in FILTER_BANK kernel#3602
Open
adilburaksen wants to merge 1 commit into
Open
Validate num_channels against tensor sizes in FILTER_BANK kernel#3602adilburaksen wants to merge 1 commit into
adilburaksen wants to merge 1 commit into
Conversation
FilterBankInit reads num_channels from the op's init flexbuffer and allocates a work area of (num_channels + 1) * sizeof(uint64_t); FilterbankAccumulateChannels then loops num_channels + 1 times, indexing channel_frequency_starts[i], channel_weight_starts[i] and channel_widths[i] and writing num_channels output elements. FilterBankPrepare validated only the rank and type of each tensor, never that num_channels is consistent with the per-channel metadata tensor lengths or the output length, so a model whose flexbuffer declares a num_channels larger than those tensors caused an out-of-bounds read. On targets where size_t is 32 bits, a large num_channels also overflows the work-area size computation into a tiny allocation that the accumulation loop then writes past (out-of-bounds write). Both were confirmed with AddressSanitizer. Reject an out-of-range num_channels in FilterBankInit, and validate in FilterBankPrepare that num_channels > 0, that the metadata tensors each have num_channels + 1 elements, and that the output has num_channels elements (mirroring the recent fix for the sibling FilterBankSpectralSubtraction kernel), plus a regression test.
adilburaksen
force-pushed
the
fix-filterbank-num-channels-oob
branch
from
a1e52b2 to
46adb06
Compare
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.
BUG=out-of-bounds read/write in FILTER_BANK kernel from unvalidated num_channels
What
The
FILTER_BANKsignal kernel takes its channel count from the op's init flexbuffer(
num_channels) and six runtime tensors.FilterBankInitreadsnum_channelsand allocates awork area of
num_channels + 1;FilterbankAccumulateChannelsthen loopsnum_channels + 1times,reading
channel_frequency_starts[i],channel_weight_starts[i]andchannel_widths[i]and writingnum_channelsoutput elements.FilterBankPreparevalidated only the rank and type of each tensor — never thatnum_channelsisconsistent with the per-channel metadata tensor lengths or the output length. A model whose flexbuffer
declares a
num_channelslarger than those tensors therefore caused an out-of-bounds read.This was confirmed with AddressSanitizer two ways:
audio_preprocessor_int8.tflite(with a single byte changed sonum_channels = 80,metadata tensors left at 41) through
GetModel()+MicroInterpreter+Invoke(), which reads offthe end of the model buffer inside
FilterbankAccumulateChannels.This is the same class of bug, and the same
num_channels-vs-tensor-size check, that was added for thesibling
FilterBankSpectralSubtractionkernel in #3594;filter_bank.ccwas not covered there.Change
filter_bank.cc:FilterBankPreparenow checksnum_channels > 0, thatchannel_frequency_starts,channel_weight_startsandchannel_widthseach havenum_channels + 1elements, and that the output hasnum_channelselements.filter_bank_test.cc: regression test that the mismatched model is rejected (no OOB).No change for valid models.