From 3e61c51e7ac35221155647816e2af6017e7d6235 Mon Sep 17 00:00:00 2001
From: Gustaf Ullberg <gustaf@webrtc.org>
Date: Wed, 27 Mar 2019 15:52:03 +0100
Subject: [PATCH] AEC3: Fix range in filter analyzer
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

This change prevents FilterAnalyzer from accessing memory out-of-bounds
when the filter is resized.

Bug: chromium:946439
Change-Id: I7e2392c8b1ff0ff55566c663bf6d7a40d7754501
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/129928
Reviewed-by: Per Åhgren <peah@webrtc.org>
Commit-Queue: Gustaf Ullberg <gustaf@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#27318}
---
 modules/audio_processing/aec3/BUILD.gn        |  1 +
 .../audio_processing/aec3/filter_analyzer.cc  | 15 ++++++---
 .../audio_processing/aec3/filter_analyzer.h   |  5 +--
 .../aec3/filter_analyzer_unittest.cc          | 31 +++++++++++++++++++
 4 files changed, 45 insertions(+), 7 deletions(-)
 create mode 100644 modules/audio_processing/aec3/filter_analyzer_unittest.cc

diff --git a/modules/audio_processing/aec3/BUILD.gn b/modules/audio_processing/aec3/BUILD.gn
index 259403c845..d41fee6531 100644
--- a/modules/audio_processing/aec3/BUILD.gn
+++ b/modules/audio_processing/aec3/BUILD.gn
@@ -207,6 +207,7 @@ if (rtc_include_tests) {
         "erl_estimator_unittest.cc",
         "erle_estimator_unittest.cc",
         "fft_data_unittest.cc",
+        "filter_analyzer_unittest.cc",
         "frame_blocker_unittest.cc",
         "main_filter_update_gain_unittest.cc",
         "matched_filter_lag_aggregator_unittest.cc",
diff --git a/modules/audio_processing/aec3/filter_analyzer.cc b/modules/audio_processing/aec3/filter_analyzer.cc
index 7a6471a5e3..6bbeb6e435 100644
--- a/modules/audio_processing/aec3/filter_analyzer.cc
+++ b/modules/audio_processing/aec3/filter_analyzer.cc
@@ -144,11 +144,16 @@ void FilterAnalyzer::SetRegionToAnalyze(
     rtc::ArrayView<const float> filter_time_domain) {
   constexpr size_t kNumberBlocksToUpdate = 1;
   auto& r = region_;
-    r.start_sample_ =
-        r.end_sample_ == filter_time_domain.size() - 1 ? 0 : r.end_sample_ + 1;
-    r.end_sample_ =
-        std::min(r.start_sample_ + kNumberBlocksToUpdate * kBlockSize - 1,
-                 filter_time_domain.size() - 1);
+  r.start_sample_ =
+      r.end_sample_ >= filter_time_domain.size() - 1 ? 0 : r.end_sample_ + 1;
+  r.end_sample_ =
+      std::min(r.start_sample_ + kNumberBlocksToUpdate * kBlockSize - 1,
+               filter_time_domain.size() - 1);
+
+  // Check range.
+  RTC_DCHECK_LT(r.start_sample_, filter_time_domain.size());
+  RTC_DCHECK_LT(r.end_sample_, filter_time_domain.size());
+  RTC_DCHECK_LE(r.start_sample_, r.end_sample_);
 }
 
 FilterAnalyzer::ConsistentFilterDetector::ConsistentFilterDetector(
diff --git a/modules/audio_processing/aec3/filter_analyzer.h b/modules/audio_processing/aec3/filter_analyzer.h
index 37d8efd284..f0f43264c7 100644
--- a/modules/audio_processing/aec3/filter_analyzer.h
+++ b/modules/audio_processing/aec3/filter_analyzer.h
@@ -55,6 +55,9 @@ class FilterAnalyzer {
   // Returns the preprocessed filter.
   rtc::ArrayView<const float> GetAdjustedFilter() const { return h_highpass_; }
 
+  // Public for testing purposes only.
+  void SetRegionToAnalyze(rtc::ArrayView<const float> filter_time_domain);
+
  private:
   void AnalyzeRegion(rtc::ArrayView<const float> filter_time_domain,
                      const RenderBuffer& render_buffer);
@@ -65,8 +68,6 @@ class FilterAnalyzer {
 
   void ResetRegion();
 
-  void SetRegionToAnalyze(rtc::ArrayView<const float> filter_time_domain);
-
   struct FilterRegion {
     size_t start_sample_;
     size_t end_sample_;
diff --git a/modules/audio_processing/aec3/filter_analyzer_unittest.cc b/modules/audio_processing/aec3/filter_analyzer_unittest.cc
new file mode 100644
index 0000000000..474d67d348
--- /dev/null
+++ b/modules/audio_processing/aec3/filter_analyzer_unittest.cc
@@ -0,0 +1,31 @@
+/*
+ *  Copyright (c) 2019 The WebRTC project authors. All Rights Reserved.
+ *
+ *  Use of this source code is governed by a BSD-style license
+ *  that can be found in the LICENSE file in the root of the source
+ *  tree. An additional intellectual property rights grant can be found
+ *  in the file PATENTS.  All contributing project authors may
+ *  be found in the AUTHORS file in the root of the source tree.
+ */
+
+#include "modules/audio_processing/aec3/filter_analyzer.h"
+
+#include <algorithm>
+
+#include "test/gmock.h"
+#include "test/gtest.h"
+
+namespace webrtc {
+
+// Verifies that the filter analyzer handles filter resizes properly.
+TEST(FilterAnalyzer, FilterResize) {
+  EchoCanceller3Config c;
+  std::vector<float> filter(65, 0.f);
+  FilterAnalyzer fa(c);
+  fa.SetRegionToAnalyze(filter);
+  fa.SetRegionToAnalyze(filter);
+  filter.resize(32);
+  fa.SetRegionToAnalyze(filter);
+}
+
+}  // namespace webrtc