Ensure that read attribute paths are valid paths.

Disallow out-of-range attribute ids and wildcard cluster with
non-wildcard non-global attribute.

Fixes #14026

src/app/InteractionModelEngine.cpp

@@ -332,7 +332,12 @@ CHIP_ERROR InteractionModelEngine::OnReadInitialRequest(Messaging::ExchangeConte
     ReadHandler * handler = mReadHandlers.CreateObject(*this, apExchangeContext, aInteractionType);
     if (handler)
     {
-        ReturnErrorOnFailure(handler->OnInitialRequest(std::move(aPayload)));
+        CHIP_ERROR err = handler->OnInitialRequest(std::move(aPayload));
+        if (err != CHIP_NO_ERROR)
+        {
+            aStatus = StatusIB(err).mStatus;
+            return err;
+        }
 
         aStatus = Protocols::InteractionModel::Status::Success;
         return CHIP_NO_ERROR;

src/app/MessageDef/StatusIB.cpp

@@ -82,7 +82,7 @@ CHIP_ERROR StatusIB::Parser::CheckSchemaValidity() const
 
 #if CHIP_DETAIL_LOGGING
             {
-                uint16_t status;
+                uint8_t status;
                 ReturnErrorOnFailure(reader.Get(status));
                 PRETTY_PRINT("\tstatus = " ChipLogFormatIMStatus ",", ChipLogValueIMStatus(static_cast<Status>(status)));
             }

src/app/MessageDef/StatusResponseMessage.cpp

@@ -16,6 +16,9 @@
 
 #include "StatusResponseMessage.h"
 #include "MessageDefHelper.h"
+#include "protocols/interaction_model/Constants.h"
+
+using namespace chip::Protocols::InteractionModel;
 
 namespace chip {
 namespace app {
@@ -42,9 +45,9 @@ CHIP_ERROR StatusResponseMessage::Parser::CheckSchemaValidity() const
             VerifyOrReturnError(TLV::kTLVType_UnsignedInteger == reader.GetType(), CHIP_ERROR_WRONG_TLV_TYPE);
 #if CHIP_DETAIL_LOGGING
             {
-                uint16_t status;
+                uint8_t status;
                 ReturnErrorOnFailure(reader.Get(status));
-                PRETTY_PRINT("\tStatus = 0x%x,", status);
+                PRETTY_PRINT("\tStatus = " ChipLogFormatIMStatus ",", ChipLogValueIMStatus(static_cast<Status>(status)));
             }
 #endif // CHIP_DETAIL_LOGGING
             break;

src/app/ReadHandler.cpp

@@ -400,46 +400,55 @@ CHIP_ERROR ReadHandler::ProcessAttributePathList(AttributePathIBs::Parser & aAtt
         AttributePathIB::Parser path;
         err = path.Init(reader);
         SuccessOrExit(err);
-        // TODO: MEIs (ClusterId and AttributeId) have a invalid pattern instead of a single invalid value, need to add separate
-        // functions for checking if we have received valid values.
-        // TODO: Wildcard cluster id with non-global attributes or wildcard attribute paths should be rejected.
+
         err = path.GetEndpoint(&(attribute.mEndpointId));
         if (err == CHIP_NO_ERROR)
         {
-            VerifyOrExit(!attribute.HasWildcardEndpointId(), err = CHIP_ERROR_IM_MALFORMED_ATTRIBUTE_PATH);
+            VerifyOrExit(!attribute.HasWildcardEndpointId(), err = CHIP_IM_GLOBAL_STATUS(InvalidAction));
         }
         else if (err == CHIP_END_OF_TLV)
         {
             err = CHIP_NO_ERROR;
         }
         SuccessOrExit(err);
-        err = path.GetCluster(&(attribute.mClusterId));
+
+        ClusterId clusterId = kInvalidClusterId;
+        err                 = path.GetCluster(&clusterId);
         if (err == CHIP_NO_ERROR)
         {
-            VerifyOrExit(!attribute.HasWildcardClusterId(), err = CHIP_ERROR_IM_MALFORMED_ATTRIBUTE_PATH);
+            VerifyOrExit(IsValidClusterId(clusterId), err = CHIP_IM_GLOBAL_STATUS(InvalidAction));
+            attribute.mClusterId = clusterId;
         }
         else if (err == CHIP_END_OF_TLV)
         {
             err = CHIP_NO_ERROR;
         }
-
         SuccessOrExit(err);
-        err = path.GetAttribute(&(attribute.mAttributeId));
+
+        AttributeId attributeId = kInvalidAttributeId;
+        err                     = path.GetAttribute(&attributeId);
         if (CHIP_END_OF_TLV == err)
         {
             err = CHIP_NO_ERROR;
         }
         else if (err == CHIP_NO_ERROR)
         {
-            VerifyOrExit(!attribute.HasWildcardAttributeId(), err = CHIP_ERROR_IM_MALFORMED_ATTRIBUTE_PATH);
+            VerifyOrExit(IsValidAttributeId(attributeId), err = CHIP_IM_GLOBAL_STATUS(InvalidAction));
+            attribute.mAttributeId = attributeId;
         }
         SuccessOrExit(err);
 
+        // A wildcard cluster requires that the attribute path either be
+        // wildcard or a global attribute.
+        VerifyOrExit(!attribute.HasWildcardClusterId() || attribute.HasWildcardAttributeId() ||
+                         IsGlobalAttribute(attribute.mAttributeId),
+                     err = CHIP_IM_GLOBAL_STATUS(InvalidAction));
+
         err = path.GetListIndex(&(attribute.mListIndex));
         if (CHIP_NO_ERROR == err)
         {
             VerifyOrExit(!attribute.HasWildcardAttributeId() && !attribute.HasWildcardListIndex(),
-                         err = CHIP_ERROR_IM_MALFORMED_ATTRIBUTE_PATH);
+                         err = CHIP_IM_GLOBAL_STATUS(InvalidAction));
         }
         else if (CHIP_END_OF_TLV == err)
         {

src/lib/core/DataModelTypes.h

@@ -57,14 +57,47 @@ static constexpr CommandId kInvalidCommandId     = 0xFFFF'FFFF;
 static constexpr EventId kInvalidEventId         = 0xFFFF'FFFF;
 static constexpr FieldId kInvalidFieldId         = 0xFFFF'FFFF;
 
+static constexpr uint16_t ExtractIdFromMEI(uint32_t aMEI)
+{
+    constexpr uint32_t kIdMask = 0x0000'FFFF;
+    return static_cast<uint16_t>(aMEI & kIdMask);
+}
+
+static constexpr uint16_t ExtractVendorFromMEI(uint32_t aMEI)
+{
+    constexpr uint32_t kVendorMask  = 0xFFFF'0000;
+    constexpr uint32_t kVendorShift = 16;
+    return static_cast<uint16_t>((aMEI & kVendorMask) >> kVendorShift);
+}
+
 constexpr bool IsValidClusterId(ClusterId aClusterId)
 {
-    const ClusterId kIdMask     = 0x0000'FFFF;
-    const ClusterId kVendorMask = 0xFFFF'0000;
-    const auto id               = aClusterId & kIdMask;
-    const auto vendor           = aClusterId & kVendorMask;
-    return (vendor == 0x0000'0000 && id <= 0x7FFF) ||
-        (vendor >= 0x0001'0000 && vendor <= 0xFFFE'0000 && id >= 0xFC00 && id <= 0xFFFE);
+    const auto id     = ExtractIdFromMEI(aClusterId);
+    const auto vendor = ExtractVendorFromMEI(aClusterId);
+    // Cluster id suffixes in the range 0x0000 to 0x7FFF indicate a standard
+    // cluster.
+    //
+    // Cluster id suffixes in the range 0xFC00 to 0xFFFE indicate an MS cluster.
+    return (vendor == 0x0000 && id <= 0x7FFF) || (vendor >= 0x0001 && vendor <= 0xFFFE && id >= 0xFC00 && id <= 0xFFFE);
+}
+
+constexpr bool IsGlobalAttribute(AttributeId aAttributeId)
+{
+    const auto id     = ExtractIdFromMEI(aAttributeId);
+    const auto vendor = ExtractVendorFromMEI(aAttributeId);
+    // Attribute id suffixes in the range 0xF000 to 0xFFFE indicate a standard
+    // global attribute.
+    return (vendor == 0x0000 && id >= 0xF000 && id <= 0xFFFE);
+}
+
+constexpr bool IsValidAttributeId(AttributeId aAttributeId)
+{
+    const auto id     = ExtractIdFromMEI(aAttributeId);
+    const auto vendor = ExtractVendorFromMEI(aAttributeId);
+    // Attribute id suffixes in the range 0x0000 to 0x4FFF indicate a non-global
+    // attribute (standard or MS).  The vendor id must not be wildcard in this
+    // case.
+    return (id <= 0x4FFF && vendor != 0xFFFF) || IsGlobalAttribute(aAttributeId);
 }
 
 constexpr bool IsValidDeviceTypeId(DeviceTypeId aDeviceTypeId)