Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
agatah2333 opened this issue Aug 1, 2024 · 4 comments · Fixed by #36542
Closed

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

agatah2333 opened this issue Aug 1, 2024 · 4 comments · Fixed by #36542
Assignees
Labels
bug Something isn't working lighting needs triage spec Mismatch between spec and implementation

Comments

@agatah2333
Copy link

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

561d23d

Platform

other, core

Platform Version(s)

1.3

Type

Common Cluster Logic, Spec Compliance Issue

Anything else?

No response

@agatah2333 agatah2333 added bug Something isn't working needs triage labels Aug 1, 2024
@bzbarsky-apple
Copy link
Contributor

@jmartinez-silabs

@jmartinez-silabs
Copy link
Member

@bzbarsky-apple Getting this pr in will help since it will add all the correct enum types.
#33612
I'll ask the author if he can finish the work.

@jmartinez-silabs jmartinez-silabs self-assigned this Aug 5, 2024
Copy link

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
Copy link
Member

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working lighting needs triage spec Mismatch between spec and implementation
Projects
3 participants