project-chip / connectedhomeip

Matter (formerly Project CHIP) creates more connections between more objects, simplifying development for manufacturers and increasing compatibility for consumers, guided by the Connectivity Standards Alliance.
https://buildwithmatter.com
Apache License 2.0
7.48k stars 2.01k forks source link

[1.3] Out-of-Range Problems in Various color control cluster #34721

Open agatah2333 opened 3 months ago

agatah2333 commented 3 months ago

Reproduction steps

When there are constraints for transaction time (0-65534) and the OptionsMask and OptionsOverride (which are restricted to 0-1) in specification, but there is no restriction in the implementation code. Thus, values such as 65535 and 3 could work normally.

  1. MoveToHue Command has an out-of-range problem There is no constraint check for field 2 (Transaction Time). The normal range is 0-65534. ./chip-tool any command-by-id 0x0300 0x00 '{"0":254,"1":2,"2":65535}' 0x654324 1

  2. MoveHue Command has an out-of-range problem There are no constraint checks for field 2 and field 3. ./chip-tool any command-by-id 0x0300 0x01 '{"0":3,"1":3,"2":3,"3":3}' 0x654324 1

  3. StepHue Command has an out-of-range problem There are no constraint checks for field 3 and field 4.

  4. MoveToSaturation Command has an out-of-range problem There are no constraint checks for field 1, field 2, and field 3. ./chip-tool any command-by-id 0x0300 0x03 '{"0":254,"1":65535,"2":3,"3":3}' 0x654324 1

  5. MoveSaturation Command has an out-of-range problem There are no constraint checks for field 2 and field 3. ./chip-tool any command-by-id 0x0300 0x04 '{"0":3,"1":1,"2":3,"3":3}' 0x654324 1

  6. StepSaturation Command has an out-of-range problem There are no constraint checks for field 3 and field 4.

  7. MoveToHueAndSaturation Command has an out-of-range problem There are no constraint checks for field 1, field 3, and field 4. ./chip-tool any command-by-id 0x0300 0x06 '{"0":254,"1":254,"2":65535,"3":3,"4":3}' 0x654324 1

  8. MoveToColor Command has an out-of-range problem There is no constraint check for field 2. ./chip-tool any command-by-id 0x0300 0x07 '{"0":65279,"1":65279,"2":65535,"3":3,"4":3}' 0x654324 1

....

Bug prevalence

each time

GitHub hash of the SDK that was being used

561d23d0db215a99705ff0696e73853c8edf11b2

Platform

other, core

Platform Version(s)

1.3

Type

Common Cluster Logic, Spec Compliance Issue

Anything else?

No response

bzbarsky-apple commented 3 months ago

@jmartinez-silabs

jmartinez-silabs commented 3 months ago

@bzbarsky-apple Getting this pr in will help since it will add all the correct enum types. https://github.com/project-chip/connectedhomeip/pull/33612 I'll ask the author if he can finish the work.

github-actions[bot] commented 1 month ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Remove stale label or comment or this will be closed in 30 days.

jmartinez-silabs commented 2 weeks ago

PR to add constraint check to the required arguments on the cluster server implementation. Note: OptionsMask and OptionsOverride are bit fields and are not constrained in the defined fields. The undefined fields are ignored by the server per spec.

It was also discussed that it was valid for Chip-tool, as a testing tool, to send out-of-range values as it can add negative tests against server implementations.