Headline
CVE-2023-42189: [BUG] KeySetRemove Command returns SUCCESS status code when GroupKeySetID 0 is being removed · Issue #28518 · project-chip/connectedhomeip
Insecure Permissions vulnerability in Connectivity Standards Alliance Matter Official SDK v.1.1.0.0 , Nanoleaf Light strip v.3.5.10, Govee LED Strip v.3.00.42, switchBot Hub2 v.1.0-0.8, Phillips hue hub v.1.59.1959097030, and yeelight smart lamp v.1.12.69 allows a remote attacker to cause a denial of service via a crafted script to the KeySetRemove function.
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
Closed
sumaky opened this issue
Aug 4, 2023
· 0 comments · Fixed by #28524
Comments
Reproduction steps
spec reference: 11.2.7.4. KeySetRemove Command
This command SHALL fail with an INVALID_COMMAND status code back to the initiator if the
GroupKeySetID being removed is 0, which is the Key Set associated with the Identity Protection Key
(IPK).
When key-set-remove command is issued with 0x0 ID we should expect INVALID_COMMAND as status code but we see SUCCESS as the status code
This issue was found during covering additional negative scenarios for TC-GRPKEY-2.2
Reference log:
GRPKEY2.2GroupKeySetID0Error.txt
Bug prevalence
NA
GitHub hash of the SDK that was being used
NA
Platform
other
Platform Version(s)
NA
Anything else?
No response
tcarmelveilleux pushed a commit to tcarmelveilleux/connectedhomeip that referenced this issue
Aug 4, 2023
Problem:
- Removing KeySet index 0 is not allowed by spec, but SDK allowed it.
- Checking that we ran without accessing fabric is not done, so error is FAILURE instead of UNSUPPORTED_ACCESS.
- Fixes project-chip#28518
This PR:
- Adds the check and tests against removing fabric index zero
- Improves error reporting for several errors in the cluster
- Combines validation logic for accessing fabric that was missing
Testing:
- Added unit tests and integration tests for additions (except for the unsupported access that requires PASE to check)
mergify bot pushed a commit that referenced this issue
Aug 10, 2023
* Fix KeySetRemove to fail on key set index 0
Problem:
- Removing KeySet index 0 is not allowed by spec, but SDK allowed it.
- Checking that we ran without accessing fabric is not done, so error is FAILURE instead of UNSUPPORTED_ACCESS.
- Fixes #28518
This PR:
- Adds the check and tests against removing fabric index zero
- Improves error reporting for several errors in the cluster
- Combines validation logic for accessing fabric that was missing
Testing:
- Added unit tests and integration tests for additions (except for the unsupported access that requires PASE to check)
* Regen tests with ZAP
* Restyled by clang-format
* Remove explicit check for undefined fabric index
* Fix build
* Rename ValidateAndGetProviderAndFabric to just GetProviderAndFabric
* Added back cleanup for VerifyOrDie argument checking
* Restyle
Co-authored-by: [email protected] [email protected] Co-authored-by: Restyled.io [email protected] Co-authored-by: Andrei Litvin [email protected] Co-authored-by: Andrei Litvin [email protected]
andy31415 added a commit that referenced this issue
Aug 10, 2023
* Fix KeySetRemove to fail on key set index 0
Problem:
- Removing KeySet index 0 is not allowed by spec, but SDK allowed it.
- Checking that we ran without accessing fabric is not done, so error is FAILURE instead of UNSUPPORTED_ACCESS.
- Fixes #28518
This PR:
- Adds the check and tests against removing fabric index zero
- Improves error reporting for several errors in the cluster
- Combines validation logic for accessing fabric that was missing
Testing:
- Added unit tests and integration tests for additions (except for the unsupported access that requires PASE to check)
* Regen tests with ZAP
* Restyled by clang-format
* Remove explicit check for undefined fabric index
* Fix build
* Rename ValidateAndGetProviderAndFabric to just GetProviderAndFabric
* Added back cleanup for VerifyOrDie argument checking
* Restyle
* Update based on code review comments
* Remove two more flight check comments
* Restyle
* Fix merge error
Co-authored-by: [email protected] [email protected] Co-authored-by: Restyled.io [email protected] Co-authored-by: Andrei Litvin [email protected]
abpoth pushed a commit to abpoth/connectedhomeip that referenced this issue
Aug 15, 2023
* Fix KeySetRemove to fail on key set index 0
Problem:
- Removing KeySet index 0 is not allowed by spec, but SDK allowed it.
- Checking that we ran without accessing fabric is not done, so error is FAILURE instead of UNSUPPORTED_ACCESS.
- Fixes project-chip#28518
This PR:
- Adds the check and tests against removing fabric index zero
- Improves error reporting for several errors in the cluster
- Combines validation logic for accessing fabric that was missing
Testing:
- Added unit tests and integration tests for additions (except for the unsupported access that requires PASE to check)
* Regen tests with ZAP
* Restyled by clang-format
* Remove explicit check for undefined fabric index
* Fix build
* Rename ValidateAndGetProviderAndFabric to just GetProviderAndFabric
* Added back cleanup for VerifyOrDie argument checking
* Restyle
Co-authored-by: [email protected] [email protected] Co-authored-by: Restyled.io [email protected] Co-authored-by: Andrei Litvin [email protected] Co-authored-by: Andrei Litvin [email protected]
abpoth pushed a commit to abpoth/connectedhomeip that referenced this issue
Aug 15, 2023
* Fix KeySetRemove to fail on key set index 0
Problem:
- Removing KeySet index 0 is not allowed by spec, but SDK allowed it.
- Checking that we ran without accessing fabric is not done, so error is FAILURE instead of UNSUPPORTED_ACCESS.
- Fixes project-chip#28518
This PR:
- Adds the check and tests against removing fabric index zero
- Improves error reporting for several errors in the cluster
- Combines validation logic for accessing fabric that was missing
Testing:
- Added unit tests and integration tests for additions (except for the unsupported access that requires PASE to check)
* Regen tests with ZAP
* Restyled by clang-format
* Remove explicit check for undefined fabric index
* Fix build
* Rename ValidateAndGetProviderAndFabric to just GetProviderAndFabric
* Added back cleanup for VerifyOrDie argument checking
* Restyle
* Update based on code review comments
* Remove two more flight check comments
* Restyle
* Fix merge error
Co-authored-by: [email protected] [email protected] Co-authored-by: Restyled.io [email protected] Co-authored-by: Andrei Litvin [email protected]
s07641069 pushed a commit to s07641069/connectedhomeip that referenced this issue
Aug 22, 2023
* Fix KeySetRemove to fail on key set index 0
Problem:
- Removing KeySet index 0 is not allowed by spec, but SDK allowed it.
- Checking that we ran without accessing fabric is not done, so error is FAILURE instead of UNSUPPORTED_ACCESS.
- Fixes project-chip#28518
This PR:
- Adds the check and tests against removing fabric index zero
- Improves error reporting for several errors in the cluster
- Combines validation logic for accessing fabric that was missing
Testing:
- Added unit tests and integration tests for additions (except for the unsupported access that requires PASE to check)
* Regen tests with ZAP
* Restyled by clang-format
* Remove explicit check for undefined fabric index
* Fix build
* Rename ValidateAndGetProviderAndFabric to just GetProviderAndFabric
* Added back cleanup for VerifyOrDie argument checking
* Restyle
Co-authored-by: [email protected] [email protected] Co-authored-by: Restyled.io [email protected] Co-authored-by: Andrei Litvin [email protected] Co-authored-by: Andrei Litvin [email protected]
s07641069 pushed a commit to s07641069/connectedhomeip that referenced this issue
Aug 22, 2023
* Fix KeySetRemove to fail on key set index 0
Problem:
- Removing KeySet index 0 is not allowed by spec, but SDK allowed it.
- Checking that we ran without accessing fabric is not done, so error is FAILURE instead of UNSUPPORTED_ACCESS.
- Fixes project-chip#28518
This PR:
- Adds the check and tests against removing fabric index zero
- Improves error reporting for several errors in the cluster
- Combines validation logic for accessing fabric that was missing
Testing:
- Added unit tests and integration tests for additions (except for the unsupported access that requires PASE to check)
* Regen tests with ZAP
* Restyled by clang-format
* Remove explicit check for undefined fabric index
* Fix build
* Rename ValidateAndGetProviderAndFabric to just GetProviderAndFabric
* Added back cleanup for VerifyOrDie argument checking
* Restyle
* Update based on code review comments
* Remove two more flight check comments
* Restyle
* Fix merge error
Co-authored-by: [email protected] [email protected] Co-authored-by: Restyled.io [email protected] Co-authored-by: Andrei Litvin [email protected]
1 participant