From 359b559dc0607865bab2d27c34b916936d4ada6c Mon Sep 17 00:00:00 2001 From: Andrew Beltrano Date: Wed, 13 Mar 2024 21:33:19 +0000 Subject: [PATCH 1/4] Fix use-after-free test instances. --- tests/unit/linux/wifi/core/TestAccessPointFactoryLinux.cxx | 6 ++++-- tests/unit/wifi/core/TestAccessPoint.cxx | 5 +++-- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/tests/unit/linux/wifi/core/TestAccessPointFactoryLinux.cxx b/tests/unit/linux/wifi/core/TestAccessPointFactoryLinux.cxx index 6fbe094d..22125432 100644 --- a/tests/unit/linux/wifi/core/TestAccessPointFactoryLinux.cxx +++ b/tests/unit/linux/wifi/core/TestAccessPointFactoryLinux.cxx @@ -1,5 +1,6 @@ #include +#include #include #include @@ -45,8 +46,9 @@ TEST_CASE("Destroy an AccessPointFactoryLinux instance", "[wifi][core][ap][linux SECTION("Destroy doesn't cause a crash") { - AccessPointFactoryLinux accessPointFactory{ std::make_unique() }; - REQUIRE_NOTHROW(accessPointFactory.~AccessPointFactoryLinux()); + std::optional accessPointFactory{}; + accessPointFactory.emplace(std::make_unique()); + REQUIRE_NOTHROW(accessPointFactory.reset()); } } diff --git a/tests/unit/wifi/core/TestAccessPoint.cxx b/tests/unit/wifi/core/TestAccessPoint.cxx index 1ca9841e..021b99ab 100644 --- a/tests/unit/wifi/core/TestAccessPoint.cxx +++ b/tests/unit/wifi/core/TestAccessPoint.cxx @@ -37,8 +37,9 @@ TEST_CASE("Destroy an AccessPoint instance", "[wifi][core][ap]") SECTION("Destroy doesn't cause a crash") { - AccessPoint accessPoint{ Test::InterfaceNameDefault, std::make_unique() }; - REQUIRE_NOTHROW(accessPoint.~AccessPoint()); + std::optional accessPoint{}; + accessPoint.emplace(Test::InterfaceNameDefault, std::make_shared()); + REQUIRE_NOTHROW(accessPoint.reset()); } } From 346ab02c68d7d0dd7714124d9705728e97c59f45 Mon Sep 17 00:00:00 2001 From: Andrew Beltrano Date: Thu, 14 Mar 2024 02:35:34 +0000 Subject: [PATCH 2/4] Fix off-by-1 with array size. --- src/linux/libnl-helpers/Netlink80211Interface.cxx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/linux/libnl-helpers/Netlink80211Interface.cxx b/src/linux/libnl-helpers/Netlink80211Interface.cxx index 30eacf81..cfebcb23 100644 --- a/src/linux/libnl-helpers/Netlink80211Interface.cxx +++ b/src/linux/libnl-helpers/Netlink80211Interface.cxx @@ -66,7 +66,7 @@ Nl80211Interface::Parse(struct nl_msg *nl80211Message) noexcept // Parse the message. std::array newInterfaceMessageAttributes{}; - int ret = nla_parse(std::data(newInterfaceMessageAttributes), std::size(newInterfaceMessageAttributes), genlmsg_attrdata(genl80211MessageHeader, 0), genlmsg_attrlen(genl80211MessageHeader, 0), nullptr); + int ret = nla_parse(std::data(newInterfaceMessageAttributes), std::size(newInterfaceMessageAttributes) - 1, genlmsg_attrdata(genl80211MessageHeader, 0), genlmsg_attrlen(genl80211MessageHeader, 0), nullptr); if (ret < 0) { LOGE << std::format("Failed to parse netlink message attributes with error {} ({})", ret, strerror(-ret)); return std::nullopt; From 67b7d12b19f8bc50c6d350bee21854326717506f Mon Sep 17 00:00:00 2001 From: Andrew Beltrano Date: Thu, 14 Mar 2024 02:35:55 +0000 Subject: [PATCH 3/4] Add explicit main for libnl-helpers unit test. --- tests/unit/linux/libnl-helpers/CMakeLists.txt | 3 ++- tests/unit/linux/libnl-helpers/Main.cxx | 16 ++++++++++++++++ 2 files changed, 18 insertions(+), 1 deletion(-) create mode 100644 tests/unit/linux/libnl-helpers/Main.cxx diff --git a/tests/unit/linux/libnl-helpers/CMakeLists.txt b/tests/unit/linux/libnl-helpers/CMakeLists.txt index 01688436..be09650f 100644 --- a/tests/unit/linux/libnl-helpers/CMakeLists.txt +++ b/tests/unit/linux/libnl-helpers/CMakeLists.txt @@ -3,6 +3,7 @@ add_executable(libnl-helpers-test-unit) target_sources(libnl-helpers-test-unit PRIVATE + Main.cxx TestNetlink80211Interface.cxx TestNetlink80211ProtocolState.cxx ) @@ -14,7 +15,7 @@ target_include_directories(libnl-helpers-test-unit target_link_libraries(libnl-helpers-test-unit PRIVATE - Catch2::Catch2WithMain + Catch2::Catch2 libnl-helpers magic_enum::magic_enum ) diff --git a/tests/unit/linux/libnl-helpers/Main.cxx b/tests/unit/linux/libnl-helpers/Main.cxx new file mode 100644 index 00000000..0952e177 --- /dev/null +++ b/tests/unit/linux/libnl-helpers/Main.cxx @@ -0,0 +1,16 @@ + +#include +#include +#include +#include +#include + +int +main(int argc, char* argv[]) +{ + static plog::ColorConsoleAppender colorConsoleAppender{}; + + plog::init(plog::debug, &colorConsoleAppender); + + return Catch::Session().run(argc, argv); +} From 94a2aa9c6e3e553dcd32ae466e76f9b6da485eaf Mon Sep 17 00:00:00 2001 From: Andrew Beltrano Date: Thu, 14 Mar 2024 02:43:52 +0000 Subject: [PATCH 4/4] Another off-by-1 with nla_parse. --- src/linux/libnl-helpers/Netlink80211WiphyBandFrequency.cxx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/linux/libnl-helpers/Netlink80211WiphyBandFrequency.cxx b/src/linux/libnl-helpers/Netlink80211WiphyBandFrequency.cxx index ac4a4805..a44f9ab8 100644 --- a/src/linux/libnl-helpers/Netlink80211WiphyBandFrequency.cxx +++ b/src/linux/libnl-helpers/Netlink80211WiphyBandFrequency.cxx @@ -27,7 +27,7 @@ WiphyBandFrequency::Parse(struct nlattr *wiphyBandFrequencyNlAttribute) noexcept { // Parse the attribute message. std::array wiphyBandFrequenciesAttributes{}; - int ret = nla_parse(std::data(wiphyBandFrequenciesAttributes), std::size(wiphyBandFrequenciesAttributes), static_cast(nla_data(wiphyBandFrequencyNlAttribute)), nla_len(wiphyBandFrequencyNlAttribute), nullptr); + int ret = nla_parse(std::data(wiphyBandFrequenciesAttributes), std::size(wiphyBandFrequenciesAttributes) - 1, static_cast(nla_data(wiphyBandFrequencyNlAttribute)), nla_len(wiphyBandFrequencyNlAttribute), nullptr); if (ret < 0) { LOGE << std::format("Failed to parse wiphy band frequency attributes with error {} ({})", ret, nl_geterror(ret)); return std::nullopt;