From 5587812b4ad0651402f623049a3e5728e2bf8e35 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Damian=20Kr=C3=B3lik?=
 <66667989+Damian-Nordic@users.noreply.github.com>
Date: Tue, 5 Apr 2022 09:05:57 +0200
Subject: [PATCH] [thread] GenericNetworkCommissioningThreadDriver error
 handling fixes (#17006)

1. Return NetworkIdNotFound instead of NetworkNotFound when
   trying to reference an unknown network.
2. Validate ReorderNetwork index.
3. Validate input dataset Init() result.
---
 ...enericNetworkCommissioningThreadDriver.cpp | 79 ++++++++-----------
 .../GenericNetworkCommissioningThreadDriver.h |  2 +
 2 files changed, 36 insertions(+), 45 deletions(-)

diff --git a/src/platform/OpenThread/GenericNetworkCommissioningThreadDriver.cpp b/src/platform/OpenThread/GenericNetworkCommissioningThreadDriver.cpp
index 153c37e055768f..ad25ce1194d8b2 100644
--- a/src/platform/OpenThread/GenericNetworkCommissioningThreadDriver.cpp
+++ b/src/platform/OpenThread/GenericNetworkCommissioningThreadDriver.cpp
@@ -73,22 +73,18 @@ CHIP_ERROR GenericThreadDriver::RevertConfiguration()
 Status GenericThreadDriver::AddOrUpdateNetwork(ByteSpan operationalDataset, MutableCharSpan & outDebugText,
                                                uint8_t & outNetworkIndex)
 {
-    uint8_t extpanid[kSizeExtendedPanId];
-    uint8_t newExtpanid[kSizeExtendedPanId];
+    ByteSpan newExtpanid;
     Thread::OperationalDataset newDataset;
 
     outDebugText.reduce_size(0);
     outNetworkIndex = 0;
 
-    newDataset.Init(operationalDataset);
-    VerifyOrReturnError(newDataset.IsCommissioned(), Status::kOutOfRange);
-
-    newDataset.GetExtendedPanId(newExtpanid);
-    mStagingNetwork.GetExtendedPanId(extpanid);
+    VerifyOrReturnError(newDataset.Init(operationalDataset) == CHIP_NO_ERROR && newDataset.IsCommissioned(), Status::kOutOfRange);
+    newDataset.GetExtendedPanIdAsByteSpan(newExtpanid);
 
     // We only support one active operational dataset. Add/Update based on either:
     // Staging network not commissioned yet (active) or we are updating the dataset with same Extended Pan ID.
-    VerifyOrReturnError(!mStagingNetwork.IsCommissioned() || memcmp(extpanid, newExtpanid, kSizeExtendedPanId) == 0,
+    VerifyOrReturnError(!mStagingNetwork.IsCommissioned() || MatchesNetworkId(mStagingNetwork, newExtpanid) == Status::kSuccess,
                         Status::kBoundsExceeded);
 
     mStagingNetwork = newDataset;
@@ -99,61 +95,37 @@ Status GenericThreadDriver::RemoveNetwork(ByteSpan networkId, MutableCharSpan &
 {
     outDebugText.reduce_size(0);
     outNetworkIndex = 0;
-    uint8_t extpanid[kSizeExtendedPanId];
-    if (!mStagingNetwork.IsCommissioned())
-    {
-        return Status::kNetworkNotFound;
-    }
-    else if (mStagingNetwork.GetExtendedPanId(extpanid) != CHIP_NO_ERROR)
-    {
-        return Status::kUnknownError;
-    }
 
-    VerifyOrReturnError(networkId.size() == kSizeExtendedPanId && memcmp(networkId.data(), extpanid, kSizeExtendedPanId) == 0,
-                        Status::kNetworkNotFound);
+    NetworkCommissioning::Status status = MatchesNetworkId(mStagingNetwork, networkId);
+
+    VerifyOrReturnError(status == Status::kSuccess, status);
     mStagingNetwork.Clear();
+
     return Status::kSuccess;
 }
 
 Status GenericThreadDriver::ReorderNetwork(ByteSpan networkId, uint8_t index, MutableCharSpan & outDebugText)
 {
     outDebugText.reduce_size(0);
-    uint8_t extpanid[kSizeExtendedPanId];
-    if (!mStagingNetwork.IsCommissioned())
-    {
-        return Status::kNetworkNotFound;
-    }
-    else if (mStagingNetwork.GetExtendedPanId(extpanid) != CHIP_NO_ERROR)
-    {
-        return Status::kUnknownError;
-    }
 
-    VerifyOrReturnError(networkId.size() == kSizeExtendedPanId && memcmp(networkId.data(), extpanid, kSizeExtendedPanId) == 0,
-                        Status::kNetworkNotFound);
+    NetworkCommissioning::Status status = MatchesNetworkId(mStagingNetwork, networkId);
+
+    VerifyOrReturnError(status == Status::kSuccess, status);
+    VerifyOrReturnError(index == 0, Status::kOutOfRange);
 
     return Status::kSuccess;
 }
 
 void GenericThreadDriver::ConnectNetwork(ByteSpan networkId, ConnectCallback * callback)
 {
-    NetworkCommissioning::Status status = Status::kSuccess;
-    uint8_t extpanid[kSizeExtendedPanId];
-    if (!mStagingNetwork.IsCommissioned())
-    {
-        ExitNow(status = Status::kNetworkNotFound);
-    }
-    else if (mStagingNetwork.GetExtendedPanId(extpanid) != CHIP_NO_ERROR)
+    NetworkCommissioning::Status status = MatchesNetworkId(mStagingNetwork, networkId);
+
+    if (status == Status::kSuccess &&
+        DeviceLayer::ThreadStackMgrImpl().AttachToThreadNetwork(mStagingNetwork.AsByteSpan(), callback) != CHIP_NO_ERROR)
     {
-        ExitNow(status = Status::kUnknownError);
+        status = Status::kUnknownError;
     }
 
-    VerifyOrExit((networkId.size() == kSizeExtendedPanId && memcmp(networkId.data(), extpanid, kSizeExtendedPanId) == 0),
-                 status = Status::kNetworkNotFound);
-
-    VerifyOrExit(DeviceLayer::ThreadStackMgrImpl().AttachToThreadNetwork(mStagingNetwork.AsByteSpan(), callback) == CHIP_NO_ERROR,
-                 status = Status::kUnknownError);
-
-exit:
     if (status != Status::kSuccess)
     {
         callback->OnResult(status, CharSpan(), 0);
@@ -175,6 +147,23 @@ void GenericThreadDriver::ScanNetworks(ThreadDriver::ScanCallback * callback)
     }
 }
 
+Status GenericThreadDriver::MatchesNetworkId(const Thread::OperationalDataset & dataset, const ByteSpan & networkId) const
+{
+    ByteSpan extpanid;
+
+    if (!dataset.IsCommissioned())
+    {
+        return Status::kNetworkIDNotFound;
+    }
+
+    if (dataset.GetExtendedPanIdAsByteSpan(extpanid) != CHIP_NO_ERROR)
+    {
+        return Status::kUnknownError;
+    }
+
+    return networkId.data_equal(extpanid) ? Status::kSuccess : Status::kNetworkIDNotFound;
+}
+
 size_t GenericThreadDriver::ThreadNetworkIterator::Count()
 {
     return driver->mStagingNetwork.IsCommissioned() ? 1 : 0;
diff --git a/src/platform/OpenThread/GenericNetworkCommissioningThreadDriver.h b/src/platform/OpenThread/GenericNetworkCommissioningThreadDriver.h
index 1fc1f1cb49eb30..6e9c71d7f03765 100644
--- a/src/platform/OpenThread/GenericNetworkCommissioningThreadDriver.h
+++ b/src/platform/OpenThread/GenericNetworkCommissioningThreadDriver.h
@@ -104,6 +104,8 @@ class GenericThreadDriver final : public ThreadDriver
     void ScanNetworks(ThreadDriver::ScanCallback * callback) override;
 
 private:
+    Status MatchesNetworkId(const Thread::OperationalDataset & dataset, const ByteSpan & networkId) const;
+
     ThreadNetworkIterator mThreadIterator      = ThreadNetworkIterator(this);
     Thread::OperationalDataset mSavedNetwork   = {};
     Thread::OperationalDataset mStagingNetwork = {};