From 20b67be01c3a97b4db54186b004550f4abc855af Mon Sep 17 00:00:00 2001 From: Henrik Lundin Date: Mon, 25 Nov 2019 10:21:00 +0100 Subject: [Backport] Security bug 1016506 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Manual backport of patch originally reviewed on https://webrtc-review.googlesource.com/c/src/+/160304: Fixing a buffer overflow in Merge::Downsample In the unlikely event that the decoded audio is really short, the downsampling would read outside of the decoded audio vector. This CL fixes that, and adds a unit test that verifies the fix (when running with ASan). Bug: chromium:1016506 Change-Id: I498b49ab4cf376d4680049fa6b0a67d7515b0e04 Reviewed-by: Jüri Valdmann --- .../webrtc/modules/audio_coding/neteq/merge.cc | 23 ++++++++++++---------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/chromium/third_party/webrtc/modules/audio_coding/neteq/merge.cc b/chromium/third_party/webrtc/modules/audio_coding/neteq/merge.cc index 357ef8dd925..552192d910a 100644 --- a/chromium/third_party/webrtc/modules/audio_coding/neteq/merge.cc +++ b/chromium/third_party/webrtc/modules/audio_coding/neteq/merge.cc @@ -286,19 +286,22 @@ void Merge::Downsample(const int16_t* input, num_coefficients, decimation_factor, kCompensateDelay); if (input_length <= length_limit) { // Not quite long enough, so we have to cheat a bit. - // If the input is really short, we'll just use the input length as is, and - // won't bother with correcting for the offset. This is clearly a - // pathological case, and the signal quality will suffer. - const size_t temp_len = input_length > signal_offset - ? input_length - signal_offset - : input_length; + // If the input is shorter than the offset, we consider the input to be 0 + // length. This will cause us to skip the downsampling since it makes no + // sense anyway, and input_downsampled_ will be filled with zeros. This is + // clearly a pathological case, and the signal quality will suffer, but + // there is not much we can do. + const size_t temp_len = + input_length > signal_offset ? input_length - signal_offset : 0; // TODO(hlundin): Should |downsamp_temp_len| be corrected for round-off // errors? I.e., (temp_len + decimation_factor - 1) / decimation_factor? size_t downsamp_temp_len = temp_len / decimation_factor; - WebRtcSpl_DownsampleFast(&input[signal_offset], temp_len, - input_downsampled_, downsamp_temp_len, - filter_coefficients, num_coefficients, - decimation_factor, kCompensateDelay); + if (downsamp_temp_len > 0) { + WebRtcSpl_DownsampleFast(&input[signal_offset], temp_len, + input_downsampled_, downsamp_temp_len, + filter_coefficients, num_coefficients, + decimation_factor, kCompensateDelay); + } memset(&input_downsampled_[downsamp_temp_len], 0, sizeof(int16_t) * (kInputDownsampLength - downsamp_temp_len)); } else { -- cgit v1.2.1