[nwprov] Resolve comments in project-chip#17038 add more checks

erjiaqing · Apr 7, 2022 · 57e3dc0 · 57e3dc0
1 parent ebcb002
commit 57e3dc0
Show file tree

Hide file tree

Showing 2 changed files with 51 additions and 4 deletions.
diff --git a/src/app/clusters/network-commissioning/network-commissioning.cpp b/src/app/clusters/network-commissioning/network-commissioning.cpp
@@ -44,6 +44,17 @@ using namespace DeviceLayer::NetworkCommissioning;
 namespace {
 // For WiFi and Thread scan results, each item will cose ~60 bytes in TLV, thus 15 is a safe upper bound of scan results.
 constexpr size_t kMaxNetworksInScanResponse = 15;
+constexpr size_t kMaxLengthOfSSID           = 32;
+constexpr size_t kMaxLengthOfCredentials    = 64;
+
+enum ValidWiFiCredentialLength
+{
+    kOpen      = 0,
+    kWEP64     = 5,
+    kMinWPAPSK = 8,
+    kMaxWPAPSK = 63,
+    kWPAPSKHex = 64,
+};
 
 NetworkCommissioningStatus ToClusterObjectEnum(Status status)
 {
@@ -304,6 +315,17 @@ void Instance::HandleScanNetworks(HandlerContext & ctx, const Commands::ScanNetw
         if (!nullableSSID.IsNull())
         {
             ssid = nullableSSID.Value();
+            if (ssid.empty())
+            {
+                // Normalize the zero value to null ByteSpan.
+                // Spec 7.17.1. Empty string is an equivalent of null.
+                ssid = ByteSpan();
+            }
+        }
+        if (!(ssid.size() <= kMaxWiFiSSIDLength))
+        {
+            ctx.mCommandHandler.AddStatus(ctx.mRequestPath, Protocols::InteractionModel::Status::InvalidCommand);
+            return;
         }
         mAsyncCommandHandle = CommandHandler::Handle(&ctx.mCommandHandler);
         mpDriver.Get<WiFiDriver *>()->ScanNetworks(ssid, this);
@@ -323,7 +345,7 @@ namespace {
 
 void FillDebugTextAndNetworkIndex(Commands::NetworkConfigResponse::Type & response, MutableCharSpan debugText, uint8_t networkIndex)
 {
-    if (debugText.size() > 0)
+    if (!debugText.empty())
     {
         response.debugText.SetValue(CharSpan(debugText.data(), debugText.size()));
     }
@@ -354,6 +376,30 @@ void Instance::HandleAddOrUpdateWiFiNetwork(HandlerContext & ctx, const Commands
 
     VerifyOrReturn(CheckFailSafeArmed(ctx));
 
+    if (req.credentials.size() == ValidWiFiCredentialLength::kOpen || req.credentials.size() == ValidWiFiCredentialLength::kWEP64 ||
+        (req.credentials.size() >= ValidWiFiCredentialLength::kMinWPAPSK &&
+         req.credentials.size() <= ValidWiFiCredentialLength::kMaxWPAPSK))
+    {
+        // Valid length, the credentials can have any characters.
+    }
+    else if (req.credentials.size() == ValidWiFiCredentialLength::kWPAPSKHex)
+    {
+        for (size_t d = 0; d < req.credentials.size(); d++)
+        {
+            if (!isxdigit(req.credentials.data()[d]))
+            {
+                ctx.mCommandHandler.AddStatus(ctx.mRequestPath, Protocols::InteractionModel::Status::InvalidCommand);
+                return;
+            }
+        }
+    }
+    else
+    {
+        // Invalid length
+        ctx.mCommandHandler.AddStatus(ctx.mRequestPath, Protocols::InteractionModel::Status::InvalidCommand);
+        return;
+    }
+
     Commands::NetworkConfigResponse::Type response;
     MutableCharSpan debugText;
 #if CHIP_CONFIG_NETWORK_COMMISSIONING_DEBUG_TEXT_BUFFER_SIZE
@@ -449,7 +495,7 @@ void Instance::OnResult(Status commissioningError, CharSpan debugText, int32_t i
 
     Commands::ConnectNetworkResponse::Type response;
     response.networkingStatus = ToClusterObjectEnum(commissioningError);
-    if (debugText.size() != 0)
+    if (!debugText.empty())
     {
         response.debugText.SetValue(debugText);
     }
@@ -492,7 +538,7 @@ void Instance::OnFinished(Status status, CharSpan debugText, ThreadScanResponseI
     TLV::TLVType listContainerType;
     ThreadScanResponse scanResponse;
     size_t networksEncoded = 0;
-    uint8_t extendedAddressBuffer[8];
+    uint8_t extendedAddressBuffer[Thread::kSizeExtendedPanId];
 
     SuccessOrExit(err = commandHandle->PrepareCommand(
                       ConcreteCommandPath(mPath.mEndpointId, NetworkCommissioning::Id, Commands::ScanNetworksResponse::Id)));

diff --git a/src/include/platform/NetworkCommissioning.h b/src/include/platform/NetworkCommissioning.h
@@ -304,7 +304,8 @@ class WiFiDriver : public Internal::WirelessDriver
      * @brief Initializes a WiFi network scan. callback->OnFinished must be called, on both success and error. Callback can
      * be called inside ScanNetworks.
      *
-     * @param ssid    The interested SSID, the scanning MAY be restricted to to the given SSID.
+     * @param ssid        The interested SSID, the scanning SHALL be restricted to to the given SSID if the ssid is not an empty
+     *                    string. And the scanning is not restricted when ssid is empty (i.e. ssid.empty() is true).
      * @param callback    Callback that will be invoked upon finishing the scan
      */
     virtual void ScanNetworks(ByteSpan ssid, ScanCallback * callback) = 0;