Check for valid ids in command and write paths.

Read/subscribe already had such checks (in AttributePathIB::Parser::ParsePath),
but commands and writes were missing them.

src/app/CommandHandler.cpp

@@ -256,13 +256,7 @@ Status CommandHandler::ProcessCommandDataIB(CommandDataIB::Parser & aCommandElem
     err = aCommandElement.GetPath(&commandPath);
     VerifyOrReturnError(err == CHIP_NO_ERROR, Status::InvalidAction);
 
-    err = commandPath.GetClusterId(&concretePath.mClusterId);
-    VerifyOrReturnError(err == CHIP_NO_ERROR, Status::InvalidAction);
-
-    err = commandPath.GetCommandId(&concretePath.mCommandId);
-    VerifyOrReturnError(err == CHIP_NO_ERROR, Status::InvalidAction);
-
-    err = commandPath.GetEndpointId(&concretePath.mEndpointId);
+    err = commandPath.GetConcreteCommandPath(concretePath);
     VerifyOrReturnError(err == CHIP_NO_ERROR, Status::InvalidAction);
 
     {
@@ -362,10 +356,7 @@ Status CommandHandler::ProcessGroupCommandDataIB(CommandDataIB::Parser & aComman
     err = aCommandElement.GetPath(&commandPath);
     VerifyOrReturnError(err == CHIP_NO_ERROR, Status::InvalidAction);
 
-    err = commandPath.GetClusterId(&clusterId);
-    VerifyOrReturnError(err == CHIP_NO_ERROR, Status::InvalidAction);
-
-    err = commandPath.GetCommandId(&commandId);
+    err = commandPath.GetGroupCommandPath(&clusterId, &commandId);
     VerifyOrReturnError(err == CHIP_NO_ERROR, Status::InvalidAction);
 
     groupId = mExchangeCtx->GetSessionHandle()->AsIncomingGroupSession()->GetGroupId();

src/app/MessageDef/AttributePathIB.cpp

@@ -174,7 +174,10 @@ CHIP_ERROR AttributePathIB::Parser::GetListIndex(DataModel::Nullable<ListIndex>
 CHIP_ERROR AttributePathIB::Parser::GetGroupAttributePath(ConcreteDataAttributePath & aAttributePath) const
 {
     ReturnErrorOnFailure(GetCluster(&aAttributePath.mClusterId));
+    VerifyOrReturnError(IsValidClusterId(aAttributePath.mClusterId), CHIP_IM_GLOBAL_STATUS(InvalidAction));
+
     ReturnErrorOnFailure(GetAttribute(&aAttributePath.mAttributeId));
+    VerifyOrReturnError(IsValidAttributeId(aAttributePath.mAttributeId), CHIP_IM_GLOBAL_STATUS(InvalidAction));
 
     CHIP_ERROR err = CHIP_NO_ERROR;
     DataModel::Nullable<ListIndex> listIndex;

src/app/MessageDef/CommandPathIB.cpp

@@ -26,6 +26,8 @@
 
 #include <app/AppConfig.h>
 
+#include <protocols/interaction_model/Constants.h>
+
 using namespace chip;
 using namespace chip::TLV;
 
@@ -114,6 +116,24 @@ CHIP_ERROR CommandPathIB::Parser::GetCommandId(chip::CommandId * const apCommand
     return GetUnsignedInteger(to_underlying(Tag::kCommandId), apCommandId);
 }
 
+CHIP_ERROR CommandPathIB::Parser::GetConcreteCommandPath(ConcreteCommandPath & aCommandPath) const
+{
+    ReturnErrorOnFailure(GetGroupCommandPath(&aCommandPath.mClusterId, &aCommandPath.mCommandId));
+
+    return GetEndpointId(&aCommandPath.mEndpointId);
+}
+
+CHIP_ERROR CommandPathIB::Parser::GetGroupCommandPath(ClusterId * apClusterId, CommandId * apCommandId) const
+{
+    ReturnErrorOnFailure(GetClusterId(apClusterId));
+    VerifyOrReturnError(IsValidClusterId(*apClusterId), CHIP_IM_GLOBAL_STATUS(InvalidAction));
+
+    ReturnErrorOnFailure(GetCommandId(apCommandId));
+    VerifyOrReturnError(IsValidCommandId(*apCommandId), CHIP_IM_GLOBAL_STATUS(InvalidAction));
+
+    return CHIP_NO_ERROR;
+}
+
 CommandPathIB::Builder & CommandPathIB::Builder::EndpointId(const chip::EndpointId aEndpointId)
 {
     // skip if error has already been set

src/app/MessageDef/CommandPathIB.h

@@ -79,6 +79,30 @@ class Parser : public ListParser
      *          #CHIP_END_OF_TLV if there is no such element
      */
     CHIP_ERROR GetCommandId(chip::CommandId * const apCommandId) const;
+
+    /**
+     * @brief Get the concrete command path, if this command path is a concrete
+     *        path.
+     *
+     * This will validate that the cluster id and command id are actually valid for a
+     * concrete path.
+     *
+     *  @param [in] aCommandPath    The command path object to write to.
+     */
+    CHIP_ERROR GetConcreteCommandPath(ConcreteCommandPath & aCommandPath) const;
+
+    /**
+     * @brief Get a group command path.
+     *
+     * This will validate that the cluster id and command id are actually valid for a
+     * group path.
+     *
+     *  @param [out] apClusterId    The cluster id in the path.
+     *  @param [out] apCommandId    The command id in the path.
+     *
+     *  @return #CHIP_NO_ERROR on success
+     */
+    CHIP_ERROR GetGroupCommandPath(ClusterId * apClusterId, CommandId * apCommandId) const;
 };
 
 class Builder : public ListBuilder

src/lib/core/DataModelTypes.h

@@ -73,6 +73,8 @@ static constexpr CommandId kInvalidCommandId     = 0xFFFF'FFFF;
 static constexpr EventId kInvalidEventId         = 0xFFFF'FFFF;
 static constexpr FieldId kInvalidFieldId         = 0xFFFF'FFFF;
 
+static constexpr uint16_t kMaxVendorId = VendorId::TestVendor4;
+
 static constexpr uint16_t ExtractIdFromMEI(uint32_t aMEI)
 {
     constexpr uint32_t kIdMask = 0x0000'FFFF;
@@ -86,6 +88,11 @@ static constexpr uint16_t ExtractVendorFromMEI(uint32_t aMEI)
     return static_cast<uint16_t>((aMEI & kVendorMask) >> kVendorShift);
 }
 
+static constexpr bool IsValidVendorId(uint16_t aVendorId)
+{
+    return aVendorId <= kMaxVendorId;
+}
+
 constexpr bool IsValidClusterId(ClusterId aClusterId)
 {
     const auto id     = ExtractIdFromMEI(aClusterId);
@@ -94,7 +101,7 @@ constexpr bool IsValidClusterId(ClusterId aClusterId)
     // 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);
+    return IsValidVendorId(vendor) && ((vendor == 0x0000 && id <= 0x7FFF) || (vendor >= 0x0001 && id >= 0xFC00 && id <= 0xFFFE));
 }
 
 constexpr bool IsGlobalAttribute(AttributeId aAttributeId)
@@ -113,7 +120,17 @@ constexpr bool IsValidAttributeId(AttributeId 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);
+    return (id <= 0x4FFF && IsValidVendorId(vendor)) || IsGlobalAttribute(aAttributeId);
+}
+
+constexpr bool IsValidCommandId(CommandId aCommandId)
+{
+    const auto id     = ExtractIdFromMEI(aCommandId);
+    const auto vendor = ExtractVendorFromMEI(aCommandId);
+
+    // Command id suffixes must be in the range 0x00 to 0xFF, and the prefix
+    // must be a valid prefix (standard or MC).
+    return id <= 0xFF && IsValidVendorId(vendor);
 }
 
 constexpr bool IsValidDeviceTypeId(DeviceTypeId aDeviceTypeId)