[Scenes] ScenesID Constraints Check (#34600)

* Added check for SceneID outside out of bounds and yaml tests * Restyled by whitespace --------- Co-authored-by: Restyled.io <[email protected]>
project-chip · Jul 29, 2024 · 144bfec · 144bfec
1 parent 725f7ac
commit 144bfec
Show file tree

Hide file tree

Showing 2 changed files with 155 additions and 7 deletions.
diff --git a/src/app/clusters/scenes-server/scenes-server.cpp b/src/app/clusters/scenes-server/scenes-server.cpp
@@ -376,9 +376,10 @@ void AddSceneParse(CommandHandlerInterface::HandlerContext & ctx, const CommandD
     response.sceneID = req.sceneID;
 
     // Verify the attributes are respecting constraints
-    if (req.transitionTime > scenes::kScenesMaxTransitionTime || req.sceneName.size() > scenes::kSceneNameMaxLength)
+    if (req.transitionTime > scenes::kScenesMaxTransitionTime || req.sceneName.size() > scenes::kSceneNameMaxLength ||
+        req.sceneID == scenes::kUndefinedSceneId)
     {
-        response.status = to_underlying(Protocols::InteractionModel::Status::InvalidCommand);
+        response.status = to_underlying(Protocols::InteractionModel::Status::ConstraintError);
         ctx.mCommandHandler.AddResponse(ctx.mRequestPath, response);
         return;
     }
@@ -483,6 +484,14 @@ void ViewSceneParse(HandlerContext & ctx, const CommandData & req, GroupDataProv
     response.groupID = req.groupID;
     response.sceneID = req.sceneID;
 
+    // Verify the attributes are respecting constraints
+    if (req.sceneID == scenes::kUndefinedSceneId)
+    {
+        response.status = to_underlying(Protocols::InteractionModel::Status::ConstraintError);
+        ctx.mCommandHandler.AddResponse(ctx.mRequestPath, response);
+        return;
+    }
+
     // Verify Endpoint in group
     VerifyOrReturn(nullptr != groupProvider);
     if (0 != req.groupID &&
@@ -830,6 +839,14 @@ void ScenesServer::HandleRemoveScene(HandlerContext & ctx, const Commands::Remov
     response.groupID = req.groupID;
     response.sceneID = req.sceneID;
 
+    // Verify the attributes are respecting constraints
+    if (req.sceneID == scenes::kUndefinedSceneId)
+    {
+        response.status = to_underlying(Protocols::InteractionModel::Status::ConstraintError);
+        ctx.mCommandHandler.AddResponse(ctx.mRequestPath, response);
+        return;
+    }
+
     // Scene Table interface data
     SceneTableEntry scene(SceneStorageId(req.sceneID, req.groupID));
 
@@ -930,6 +947,14 @@ void ScenesServer::HandleStoreScene(HandlerContext & ctx, const Commands::StoreS
     response.groupID = req.groupID;
     response.sceneID = req.sceneID;
 
+    // Verify the attributes are respecting constraints
+    if (req.sceneID == scenes::kUndefinedSceneId)
+    {
+        response.status = to_underlying(Protocols::InteractionModel::Status::ConstraintError);
+        ctx.mCommandHandler.AddResponse(ctx.mRequestPath, response);
+        return;
+    }
+
     CHIP_ERROR err = StoreSceneParse(ctx.mCommandHandler.GetAccessingFabricIndex(), ctx.mRequestPath.mEndpointId, req.groupID,
                                      req.sceneID, mGroupProvider);
 
@@ -943,6 +968,14 @@ void ScenesServer::HandleStoreScene(HandlerContext & ctx, const Commands::StoreS
 void ScenesServer::HandleRecallScene(HandlerContext & ctx, const Commands::RecallScene::DecodableType & req)
 {
     MATTER_TRACE_SCOPE("RecallScene", "Scenes");
+
+    // Verify the attributes are respecting constraints
+    if (req.sceneID == scenes::kUndefinedSceneId)
+    {
+        ctx.mCommandHandler.AddStatus(ctx.mRequestPath, Protocols::InteractionModel::Status::ConstraintError);
+        return;
+    }
+
     CHIP_ERROR err = RecallSceneParse(ctx.mCommandHandler.GetAccessingFabricIndex(), ctx.mRequestPath.mEndpointId, req.groupID,
                                       req.sceneID, req.transitionTime, mGroupProvider);
 
@@ -1025,6 +1058,14 @@ void ScenesServer::HandleCopyScene(HandlerContext & ctx, const Commands::CopySce
     response.groupIdentifierFrom = req.groupIdentifierFrom;
     response.sceneIdentifierFrom = req.sceneIdentifierFrom;
 
+    // Verify the attributes are respecting constraints
+    if (req.sceneIdentifierFrom == scenes::kUndefinedSceneId || req.sceneIdentifierTo == scenes::kUndefinedSceneId)
+    {
+        response.status = to_underlying(Protocols::InteractionModel::Status::ResourceExhausted);
+        ctx.mCommandHandler.AddResponse(ctx.mRequestPath, response);
+        return;
+    }
+
     // Verify Endpoint in group
     VerifyOrReturn(nullptr != mGroupProvider);
     if ((0 != req.groupIdentifierFrom &&

diff --git a/src/app/tests/suites/certification/Test_TC_S_2_2.yaml b/src/app/tests/suites/certification/Test_TC_S_2_2.yaml
@@ -302,6 +302,27 @@ tests:
               - name: "SceneID"
                 value: 0x01
 
+    - label:
+          "Step 2d: TH sends a StoreScene command to DUT with the GroupID field
+          set to G1 and the SceneID field set to 0xFF, which is outside of the
+          constraints for a SceneID."
+      PICS: S.S.C04.Rsp
+      command: "StoreScene"
+      arguments:
+          values:
+              - name: "GroupID"
+                value: GI
+              - name: "SceneID"
+                value: 0xFF
+      response:
+          values:
+              - name: "Status"
+                value: 0x87
+              - name: "GroupID"
+                value: GI
+              - name: "SceneID"
+                value: 0xFF
+
     - label:
           "Step 3a: TH configures AC2 on DUT for all implemented application
           clusters supporting scenes."
@@ -444,6 +465,21 @@ tests:
               - name: "SceneID"
                 value: 0x01
 
+    - label:
+          "Step 4e: TH sends a RecallScene command to DUT with the GroupID field
+          set to G1 and the SceneID field set to 0xFF, which is outside of the
+          constraints for a SceneID."
+      PICS: S.S.C05.Rsp
+      command: "RecallScene"
+      arguments:
+          values:
+              - name: "GroupID"
+                value: G1
+              - name: "SceneID"
+                value: 0xFF
+      response:
+          error: CONSTRAINT_ERROR
+
     - label:
           "Step 5a: TH sends a ViewScene command to DUT with the GroupID field
           set to G1 and the SceneID field set to 0x01."
@@ -555,6 +591,27 @@ tests:
               - name: "SceneID"
                 value: 0xFE
 
+    - label:
+          "Step 5d: TH sends a ViewScene command to DUT with the GroupID field
+          set to G1 and the SceneID field set to 0xFF, which is outside of the
+          constraints for a SceneID."
+      PICS: S.S.C01.Rsp
+      command: "ViewScene"
+      arguments:
+          values:
+              - name: "GroupID"
+                value: G1
+              - name: "SceneID"
+                value: 0xFF
+      response:
+          values:
+              - name: "Status"
+                value: 0x87
+              - name: "GroupID"
+                value: G1
+              - name: "SceneID"
+                value: 0xFF
+
     - label:
           "Step 6: TH sends a GetSceneMembership command to DUT with the GroupID
           field set to G1."
@@ -734,7 +791,7 @@ tests:
           "Step 8d: TH sends a AddScene command to DUT with the GroupID field
           set to G1, the SceneID field set to 0x01, the TransitionTime field set
           to 70 000 000 (70 000s) and no extension field sets. This should fail
-          and return a status of 0x85 (INVALID_COMMAND)."
+          and return a status of 0x87 (CONSTRAINT_ERROR)."
       PICS: S.S.C00.Rsp
       command: "AddScene"
       arguments:
@@ -752,7 +809,7 @@ tests:
       response:
           values:
               - name: "Status"
-                value: 0x85
+                value: 0x87
               - name: "GroupID"
                 value: G1
               - name: "SceneID"
@@ -762,7 +819,7 @@ tests:
           "Step 8e: TH sends a AddScene command to DUT with the GroupID field
           set to G1, the SceneID field set to 0x01, the TransitionTime field set
           to 60 000 001 (60 000.001s) and no extension field sets. This should
-          fail and return a status of 0x85 (INVALID_COMMAND)."
+          fail and return a status of 0x87 (CONSTRAINT_ERROR)."
       PICS: S.S.C00.Rsp
       command: "AddScene"
       arguments:
@@ -780,12 +837,41 @@ tests:
       response:
           values:
               - name: "Status"
-                value: 0x85
+                value: 0x87
               - name: "GroupID"
                 value: G1
               - name: "SceneID"
                 value: 0x01
 
+    - label:
+          "Step 8f: TH sends a AddScene command to DUT with the GroupID field
+          set to G1, the SceneID field set to 0xFF, which is outside of the
+          constraints for a SceneID, the TransitionTime field set to 1000 (1s)
+          and no extension field sets. This should fail and return a status of
+          0x87 (CONSTRAINT_ERROR)."
+      PICS: S.S.C00.Rsp
+      command: "AddScene"
+      arguments:
+          values:
+              - name: "GroupID"
+                value: G1
+              - name: "SceneID"
+                value: 0xFF
+              - name: "TransitionTime"
+                value: 1000
+              - name: "SceneName"
+                value: "Scene1"
+              - name: "ExtensionFieldSets"
+                value: []
+      response:
+          values:
+              - name: "Status"
+                value: 0x87
+              - name: "GroupID"
+                value: G1
+              - name: "SceneID"
+                value: 0xFF
+
     - label:
           "Step 9a: TH sends a RemoveScene command to DUT with the GroupID field
           set to G1 and the SceneID field set to 0x01."
@@ -848,7 +934,28 @@ tests:
                 value: 0x01
 
     - label:
-          "Step 9d: TH sends a GetSceneMembership command to DUT with the
+          "Step 9d: TH sends a RemoveScene command to DUT with the GroupID field
+          set to G1 and the SceneID field set to 0xFF, which is outside of the
+          constraints for a SceneID."
+      PICS: S.S.C02.Rsp
+      command: "RemoveScene"
+      arguments:
+          values:
+              - name: "GroupID"
+                value: GI
+              - name: "SceneID"
+                value: 0xFF
+      response:
+          values:
+              - name: "Status"
+                value: 0x87
+              - name: "GroupID"
+                value: GI
+              - name: "SceneID"
+                value: 0xFF
+
+    - label:
+          "Step 9e: TH sends a GetSceneMembership command to DUT with the
           GroupID field set to G1."
       PICS: S.S.C06.Rsp
       command: "GetSceneMembership"