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.

Sign up for GitHub

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

Jump to bottom

Defaults for cluster objects #10271

Closed
yunhanw-google opened this issue Oct 6, 2021 · 2 comments · Fixed by #13432
Closed

Defaults for cluster objects #10271

yunhanw-google opened this issue Oct 6, 2021 · 2 comments · Fixed by #13432
Assignees
@bzbarsky-apple
Labels
Interaction Model Work p1 priority 1 work

Comments

@yunhanw-google
Copy link
Contributor

So comparing this to our existing decoding code I see the following differences:

  1. Our existing code ensures that all mandatory command arguments are present, or at least tries to. I think it might be assuming all arguments are mandatory. This is important, because our members don't have any initializers, generally, so if there are missing fields we will have un-initialized data in the struct. Or we need to add initialization to some sort of (XML-defined?) default values.... Or something.
  2. Our existing code errors out if the same field id is present twice.

Originally posted by @bzbarsky-apple in #10171 (comment)
@yunhanw-google yunhanw-google mentioned this issue Oct 6, 2021
Merged
@bzbarsky-apple
Copy link
Contributor

This is a critical security issue: without this, some command payloads will cause us to read uninitialized memory and do ... something.

@mrjerryjohns mrjerryjohns changed the title Initializer for cluster object codegen Defaults for cluster objects Dec 6, 2021
@mrjerryjohns
Copy link
Contributor

Seems like @bzbarsky-apple took an initial crack at this, so assigning this his way.

bzbarsky-apple added a commit to bzbarsky-apple/connectedhomeip that referenced this issue Jan 11, 2022
@bzbarsky-apple
Ensure all fields in cluster-objects structs are default-initialized.
aca5d82 
Otherwise we can end up reading random values from un-initialized
memory (e.g. if a mandatory command/struct/event field is not sent by
the other side).

Fixes project-chip#10271
@bzbarsky-apple bzbarsky-apple mentioned this issue Jan 11, 2022
Merged
bzbarsky-apple added a commit to bzbarsky-apple/connectedhomeip that referenced this issue Jan 11, 2022
@bzbarsky-apple
Ensure all fields in cluster-objects structs are default-initialized.
2087c91 
Otherwise we can end up reading random values from un-initialized
memory (e.g. if a mandatory command/struct/event field is not sent by
the other side).

Fixes project-chip#10271
bzbarsky-apple added a commit to bzbarsky-apple/connectedhomeip that referenced this issue Jan 13, 2022
@bzbarsky-apple
Ensure all fields in cluster-objects structs are default-initialized.
190ed2d 
Otherwise we can end up reading random values from un-initialized
memory (e.g. if a mandatory command/struct/event field is not sent by
the other side).

Fixes project-chip#10271
andy31415 pushed a commit that referenced this issue Jan 13, 2022
@bzbarsky-apple
Ensure all fields in cluster-objects structs are default-initialized. (…
0414c8f 
…#13432)

Otherwise we can end up reading random values from un-initialized
memory (e.g. if a mandatory command/struct/event field is not sent by
the other side).

Fixes #10271
selissia pushed a commit to selissia/connectedhomeip that referenced this issue Jan 28, 2022
@bzbarsky-apple @selissia
Ensure all fields in cluster-objects structs are default-initialized. (…
a98fb2c 
…project-chip#13432)

Otherwise we can end up reading random values from un-initialized
memory (e.g. if a mandatory command/struct/event field is not sent by
the other side).

Fixes project-chip#10271
step0035 pushed a commit to hank820/connectedhomeip that referenced this issue Feb 8, 2022
@bzbarsky-apple @step0035
Ensure all fields in cluster-objects structs are default-initialized. (…
93625f2 
…project-chip#13432)

Otherwise we can end up reading random values from un-initialized
memory (e.g. if a mandatory command/struct/event field is not sent by
the other side).

Fixes project-chip#10271
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@bzbarsky-apple bzbarsky-apple

Labels
Interaction Model Work p1 priority 1 work
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Ensure all fields in cluster-objects structs are default-initialized. bzbarsky-apple/connectedhomeip
3 participants
@mrjerryjohns @yunhanw-google @bzbarsky-apple