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

[TLV] TLVReader should take care of the range of integer values #5152

Closed
erjiaqing opened this issue Mar 4, 2021 · 0 comments · Fixed by #9944
Closed

[TLV] TLVReader should take care of the range of integer values #5152

erjiaqing opened this issue Mar 4, 2021 · 0 comments · Fixed by #9944
Assignees
@bitkis

Comments

@erjiaqing
Copy link
Contributor

Problem

The reader class should take care of this.

It currently does not.

I think it is ok if we ask to read a int64 value but only gives a int32 variable when the value is in int32 range actually?

Sure, but the problems start when we ask to read an int8 value but the TLV contains a value that does not actually fit in 8 bits. That read will succeed, which is broken. You're right that we should solve this in the TLV reader in general, but no one has had time to do that yet.

Proposed solution

Check the type of the value, and return error of it if too big.

Originally posted by @bzbarsky-apple in #5032 (comment)
@erjiaqing erjiaqing mentioned this issue Mar 4, 2021
Merged
@bzbarsky-apple bzbarsky-apple changed the title [TLV] TLVReader should take care of the range of values [TLV] TLVReader should take care of the range of integer values Mar 4, 2021
bitkis added a commit to bitkis/connectedhomeip that referenced this issue Mar 10, 2021
@bitkis
Add verification of the range of integer values for TLVReader. Resolves
4d700eb 
project-chip#5152

Signed-off-by: Boris Itkis <[email protected]>
@bitkis bitkis mentioned this issue Mar 10, 2021
Closed
bitkis added a commit to bitkis/connectedhomeip that referenced this issue Mar 12, 2021
@bitkis
Add verification of the range of integer values for TLVReader. Resolves
665ac4f 
project-chip#5152
bitkis added a commit to bitkis/connectedhomeip that referenced this issue Apr 2, 2021
@bitkis
Add verification of the range of integer values for TLVReader. Resolves
14d8c4a 
project-chip#5152
@bitkis bitkis mentioned this issue Apr 2, 2021
Closed
@bzbarsky-apple bzbarsky-apple mentioned this issue Jul 22, 2021
Merged
bzbarsky-apple added a commit to bzbarsky-apple/connectedhomeip that referenced this issue Sep 24, 2021
@bzbarsky-apple
Ensure that attempts to read integers from TLV don't silently produce…
11e8ede 
… unexpected values.

Two changes:

1) Ensure the value was encoded with the signed-ness the reader expects.

2) Ensure the value is in the range the reader expects instad of
   ending up with overflow and whatever that does..

Fixes project-chip#5152

The test changes were largely lifted from
project-chip#5764
@bzbarsky-apple bzbarsky-apple mentioned this issue Sep 24, 2021
Merged
woody-apple pushed a commit that referenced this issue Sep 28, 2021
@bzbarsky-apple
Ensure that attempts to read integers from TLV don't silently produce…
f578417 
… unexpected values. (#9944)

Two changes:

1) Ensure the value was encoded with the signed-ness the reader expects.

2) Ensure the value is in the range the reader expects instad of
   ending up with overflow and whatever that does..

Fixes #5152

The test changes were largely lifted from
#5764
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@bitkis bitkis

Labels
None yet
Projects
None yet
Milestone
No milestone
2 participants
@erjiaqing @bitkis