[wlanif] Fix handling of large rate array
Fix handling when more than WLAN_MAC_MAX_RATES rates are passed to
ConvertRateSets. Instead of reading outside the bounds of the rate
array and allocating memory indefinitely, it now restricts operation
to the maximum allowable number of rates (and asserts in a debug
build).
WLAN-1135
TEST: Verified manually using the reproduction instructions from
WLAN-1135 (replaying invalid frames via scapy). This patch
also adds a new unit test for the case.
Change-Id: I666a4819cf6b27c76ec75fbe1cab1580db875da1
diff --git a/src/connectivity/wlan/drivers/wlanif/BUILD.gn b/src/connectivity/wlan/drivers/wlanif/BUILD.gn
index b0be081..cc3b689 100644
--- a/src/connectivity/wlan/drivers/wlanif/BUILD.gn
+++ b/src/connectivity/wlan/drivers/wlanif/BUILD.gn
@@ -5,35 +5,46 @@
import("//build/config/fuchsia/rules.gni")
import("//build/package.gni")
-source_set("source") {
- public = [
- "convert.h",
- "device.h",
- "driver.h",
- ]
+all_hdrs = [
+ "convert.h",
+ "device.h",
+ "driver.h",
+]
- sources = [
- "convert.cpp",
- "device.cpp",
- "driver.cpp",
- ]
+all_src = [
+ "convert.cpp",
+ "device.cpp",
+ "driver.cpp"
+]
- public_deps = [
- "//garnet/lib/wlan/fidl",
- "//garnet/lib/wlan/protocol",
- "//src/connectivity/wlan/lib/common/cpp:common",
- "//zircon/public/lib/async-cpp",
- "//zircon/public/lib/async-loop-cpp",
- "//zircon/public/lib/ddk",
- "//zircon/public/lib/driver",
- ]
+all_deps = [
+ "//garnet/lib/wlan/fidl",
+ "//garnet/lib/wlan/protocol",
+ "//src/connectivity/wlan/lib/common/cpp:common",
+ "//zircon/public/lib/async-cpp",
+ "//zircon/public/lib/async-loop-cpp",
+ "//zircon/public/lib/ddk",
+ "//zircon/public/lib/driver",
+]
+
+source_set("lib_source") {
+ public = all_hdrs
+ sources = all_src
+ public_deps = all_deps
+}
+
+source_set("test_source") {
+ defines = [ "WLANIF_TEST=1" ]
+ public = all_hdrs
+ sources = all_src
+ public_deps = all_deps
}
driver_module("wlanif_driver") {
output_name = "wlanif"
deps = [
- ":source",
+ ":lib_source",
]
configs -= [ "//build/config/fuchsia:no_cpp_standard_library" ]
diff --git a/src/connectivity/wlan/drivers/wlanif/convert.cpp b/src/connectivity/wlan/drivers/wlanif/convert.cpp
index f9e5430..b5a4b9a 100644
--- a/src/connectivity/wlan/drivers/wlanif/convert.cpp
+++ b/src/connectivity/wlan/drivers/wlanif/convert.cpp
@@ -219,12 +219,21 @@
(*basic).resize(0);
(*op).resize(0);
+ uint16_t total_rate_count = wlanif_desc.num_rates;
+ if (total_rate_count > WLAN_MAC_MAX_RATES) {
+#if !WLANIF_TEST
+ warnf("num_rates is %u > max allowed size: %d\n", total_rate_count, WLAN_MAC_MAX_RATES);
+ ZX_DEBUG_ASSERT(total_rate_count <= WLAN_MAC_MAX_RATES);
+#endif
+ total_rate_count = WLAN_MAC_MAX_RATES;
+ }
+
// TODO(eyw): Use WlanRate data structure when it is available.
constexpr uint8_t kBasicRateMask = 0b10000000;
constexpr uint8_t kHalfMbpsMask = 0b01111111;
- for (uint8_t i = 0; i < wlanif_desc.num_rates; ++i) {
+ for (uint16_t i = 0; i < total_rate_count; ++i) {
uint8_t rate = wlanif_desc.rates[i];
if (rate & kBasicRateMask) { basic->push_back(rate & kHalfMbpsMask); }
op->push_back(rate & kHalfMbpsMask);
diff --git a/src/connectivity/wlan/drivers/wlanif/convert.h b/src/connectivity/wlan/drivers/wlanif/convert.h
index 3a2d18c..9ff0e761 100644
--- a/src/connectivity/wlan/drivers/wlanif/convert.h
+++ b/src/connectivity/wlan/drivers/wlanif/convert.h
@@ -48,5 +48,7 @@
void ConvertIfaceStats(::fuchsia::wlan::stats::IfaceStats* fidl_stats, const wlanif_stats_t& stats);
uint32_t ConvertMgmtCaptureFlags(::fuchsia::wlan::mlme::MgmtFrameCaptureFlags fidl_flags);
::fuchsia::wlan::mlme::MgmtFrameCaptureFlags ConvertMgmtCaptureFlags(uint32_t ddk_flags);
+void ConvertRateSets(::std::vector<uint8_t>* basic, ::std::vector<uint8_t>* op,
+ const wlanif_bss_description_t& wlanif_desc);
} // namespace wlanif
diff --git a/src/connectivity/wlan/drivers/wlanif/test/BUILD.gn b/src/connectivity/wlan/drivers/wlanif/test/BUILD.gn
index 7d13c34..118d691 100644
--- a/src/connectivity/wlan/drivers/wlanif/test/BUILD.gn
+++ b/src/connectivity/wlan/drivers/wlanif/test/BUILD.gn
@@ -59,7 +59,7 @@
]
deps = [
- "//src/connectivity/wlan/drivers/wlanif:source",
+ "//src/connectivity/wlan/drivers/wlanif:test_source",
"//src/connectivity/wlan/lib/mlme/cpp:mlme",
"//third_party/googletest:gtest_main",
]
diff --git a/src/connectivity/wlan/drivers/wlanif/test/convert_unittest.cpp b/src/connectivity/wlan/drivers/wlanif/test/convert_unittest.cpp
index ca35174..21f5ed1 100644
--- a/src/connectivity/wlan/drivers/wlanif/test/convert_unittest.cpp
+++ b/src/connectivity/wlan/drivers/wlanif/test/convert_unittest.cpp
@@ -62,5 +62,24 @@
EXPECT_EQ(status, ZX_OK);
}
+TEST(ConvertTest, ToVectorRateSets_InvalidRateCount) {
+ wlanif_bss_description bss_desc = {};
+
+ constexpr uint8_t kBasicRateMask = 0b10000000;
+ size_t basic_rate_count = 0;
+
+ bss_desc.num_rates = WLAN_MAC_MAX_RATES + 1;
+ for (unsigned i = 0; i < WLAN_MAC_MAX_RATES; i++) {
+ bss_desc.rates[i] = i;
+ if (i & kBasicRateMask) { basic_rate_count++; }
+ }
+ std::vector<uint8_t> basic_rates;
+ std::vector<uint8_t> op_rates;
+ ConvertRateSets(&basic_rates, &op_rates, bss_desc);
+
+ EXPECT_EQ(basic_rates.size(), basic_rate_count);
+ EXPECT_EQ(op_rates.size(), (size_t)WLAN_MAC_MAX_RATES);
+}
+
} // namespace
} // namespace wlanif
