From 04789a380f1e52d80533f0367b7ae88dd9a4212a Mon Sep 17 00:00:00 2001 From: Alami-Amine Date: Tue, 17 Sep 2024 20:39:32 +0200 Subject: [PATCH 1/5] Fuzzing different Transport Types for all-clusters-app --- .../all-clusters-app/linux/fuzzing-main.cpp | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/examples/all-clusters-app/linux/fuzzing-main.cpp b/examples/all-clusters-app/linux/fuzzing-main.cpp index 793a70a6a9fa3e..44b58786b71bd0 100644 --- a/examples/all-clusters-app/linux/fuzzing-main.cpp +++ b/examples/all-clusters-app/linux/fuzzing-main.cpp @@ -73,7 +73,11 @@ extern "C" int LLVMFuzzerTestOneInput(const uint8_t * aData, size_t aSize) // For now, just dump the data as a UDP payload into the session manager. // But maybe we should try to separately extract a PeerAddress and data from // the incoming data? - Transport::PeerAddress peerAddr; + + // dumping payload with random transport types + Transport::Type fuzzedTransportType = static_cast(*aData % 5); + Transport::PeerAddress peerAddr(fuzzedTransportType); + System::PacketBufferHandle buf = System::PacketBufferHandle::NewWithData(aData, aSize, /* aAdditionalSize = */ 0, /* aReservedSize = */ 0); if (buf.IsNull()) @@ -84,8 +88,17 @@ extern "C" int LLVMFuzzerTestOneInput(const uint8_t * aData, size_t aSize) // Ignoring the return value from OnMessageReceived, because we might be // passing it all sorts of garbage that will cause it to fail. - Server::GetInstance().GetSecureSessionManager().OnMessageReceived(peerAddr, std::move(buf)); + // for TCP we need to have MessageTransportContext + if (fuzzedTransportType == Transport::Type::kTcp) + { + Transport::MessageTransportContext msgContext; + Server::GetInstance().GetSecureSessionManager().OnMessageReceived(peerAddr, std::move(buf), &msgContext); + } + else + { + Server::GetInstance().GetSecureSessionManager().OnMessageReceived(peerAddr, std::move(buf)); + } // Now process pending events until our sentinel is reached. PlatformMgr().ScheduleWork([](intptr_t) { PlatformMgr().StopEventLoopTask(); }); PlatformMgr().RunEventLoop(); From b46ce9f110057d1f6399c8431e42ac61a2e402a4 Mon Sep 17 00:00:00 2001 From: Alami-Amine Date: Wed, 18 Sep 2024 12:55:29 +0200 Subject: [PATCH 2/5] Adding an enum value for the number of transport types --- src/transport/raw/PeerAddress.h | 1 + 1 file changed, 1 insertion(+) diff --git a/src/transport/raw/PeerAddress.h b/src/transport/raw/PeerAddress.h index 896648f76f537a..d58bf3caaedf78 100644 --- a/src/transport/raw/PeerAddress.h +++ b/src/transport/raw/PeerAddress.h @@ -54,6 +54,7 @@ enum class Type : uint8_t kBle, kTcp, kWiFiPAF, + kLast = kWiFiPAF, //This is not an actual transport type, it just refers to the last transport type }; /** From a3d0fb712a681cf99ca1614d3b85e1ec6ea76169 Mon Sep 17 00:00:00 2001 From: Alami-Amine Date: Wed, 18 Sep 2024 12:57:03 +0200 Subject: [PATCH 3/5] 1. replacing magic number when fuzzing the number of transport types 2. using different parts of the fuzzed input data for TransportType and for Payload --- examples/all-clusters-app/linux/fuzzing-main.cpp | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/examples/all-clusters-app/linux/fuzzing-main.cpp b/examples/all-clusters-app/linux/fuzzing-main.cpp index 44b58786b71bd0..ba7f792a098907 100644 --- a/examples/all-clusters-app/linux/fuzzing-main.cpp +++ b/examples/all-clusters-app/linux/fuzzing-main.cpp @@ -74,12 +74,18 @@ extern "C" int LLVMFuzzerTestOneInput(const uint8_t * aData, size_t aSize) // But maybe we should try to separately extract a PeerAddress and data from // the incoming data? - // dumping payload with random transport types - Transport::Type fuzzedTransportType = static_cast(*aData % 5); + // dumping payload with fuzzed transport types + constexpr uint8_t numberOfTypes = static_cast(Transport::Type::kLast) + 1; + Transport::Type fuzzedTransportType = static_cast(aData[0] % numberOfTypes); Transport::PeerAddress peerAddr(fuzzedTransportType); + if (aSize < 1) + { + return 0; + } + System::PacketBufferHandle buf = - System::PacketBufferHandle::NewWithData(aData, aSize, /* aAdditionalSize = */ 0, /* aReservedSize = */ 0); + System::PacketBufferHandle::NewWithData(&aData[1], aSize - 1, /* aAdditionalSize = */ 0, /* aReservedSize = */ 0); if (buf.IsNull()) { // Too big; we couldn't represent this as a packetbuffer to start with. From 8f17d3b27921850f632db786526939752177d51a Mon Sep 17 00:00:00 2001 From: "Restyled.io" Date: Wed, 18 Sep 2024 14:27:51 +0000 Subject: [PATCH 4/5] Restyled by clang-format --- examples/all-clusters-app/linux/fuzzing-main.cpp | 2 +- src/transport/raw/PeerAddress.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/examples/all-clusters-app/linux/fuzzing-main.cpp b/examples/all-clusters-app/linux/fuzzing-main.cpp index ba7f792a098907..cfca923adcffe0 100644 --- a/examples/all-clusters-app/linux/fuzzing-main.cpp +++ b/examples/all-clusters-app/linux/fuzzing-main.cpp @@ -75,7 +75,7 @@ extern "C" int LLVMFuzzerTestOneInput(const uint8_t * aData, size_t aSize) // the incoming data? // dumping payload with fuzzed transport types - constexpr uint8_t numberOfTypes = static_cast(Transport::Type::kLast) + 1; + constexpr uint8_t numberOfTypes = static_cast(Transport::Type::kLast) + 1; Transport::Type fuzzedTransportType = static_cast(aData[0] % numberOfTypes); Transport::PeerAddress peerAddr(fuzzedTransportType); diff --git a/src/transport/raw/PeerAddress.h b/src/transport/raw/PeerAddress.h index d58bf3caaedf78..60d92b8f7b5d04 100644 --- a/src/transport/raw/PeerAddress.h +++ b/src/transport/raw/PeerAddress.h @@ -54,7 +54,7 @@ enum class Type : uint8_t kBle, kTcp, kWiFiPAF, - kLast = kWiFiPAF, //This is not an actual transport type, it just refers to the last transport type + kLast = kWiFiPAF, // This is not an actual transport type, it just refers to the last transport type }; /** From fec7b495dfa6cc6db13ec7081ce2817eadf07f8d Mon Sep 17 00:00:00 2001 From: Alami-Amine Date: Thu, 19 Sep 2024 13:02:01 +0200 Subject: [PATCH 5/5] avoiding out of bounds access --- examples/all-clusters-app/linux/fuzzing-main.cpp | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/examples/all-clusters-app/linux/fuzzing-main.cpp b/examples/all-clusters-app/linux/fuzzing-main.cpp index cfca923adcffe0..2d8422d0d2eee6 100644 --- a/examples/all-clusters-app/linux/fuzzing-main.cpp +++ b/examples/all-clusters-app/linux/fuzzing-main.cpp @@ -74,16 +74,17 @@ extern "C" int LLVMFuzzerTestOneInput(const uint8_t * aData, size_t aSize) // But maybe we should try to separately extract a PeerAddress and data from // the incoming data? + // To avoid out-of-bounds access when acessing aData[1] + if (aSize < 2) + { + return 0; + } + // dumping payload with fuzzed transport types constexpr uint8_t numberOfTypes = static_cast(Transport::Type::kLast) + 1; Transport::Type fuzzedTransportType = static_cast(aData[0] % numberOfTypes); Transport::PeerAddress peerAddr(fuzzedTransportType); - if (aSize < 1) - { - return 0; - } - System::PacketBufferHandle buf = System::PacketBufferHandle::NewWithData(&aData[1], aSize - 1, /* aAdditionalSize = */ 0, /* aReservedSize = */ 0); if (buf.IsNull())