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

Reset buffer size indicators that are too large and check for maximum size on marshalling #223

Merged
merged 2 commits into from Jun 23, 2021

Conversation

stefanberger
Copy link
Owner

Reset buffer size indicators that are found to be too large so that we don't have bad buffer sizes in memory that can cause issues when marshaling the data upon saving volatile data for example.
Check for the maximum size of a buffer when marshaling data so we do not overstep the TPM2B's buffer size.

Reset the buffer size indicator in a TPM2B type of buffer after it failed
the test for the maximum buffer size it allows. This prevents having bad
buffer sizes in memory that can come to haunt us when writing the volatile
state for example.

Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
Add maxSize parameter to TPM2B_Marshal and assert on it checking
the size of the data intended to be marshaled versus the maximum
buffer size.

Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
@coveralls
Copy link

Pull Request Test Coverage Report for Build 2060

  • 36 of 43 (83.72%) changed or added relevant lines in 3 files are covered.
  • 1 unchanged line in 1 file lost coverage.
  • Overall coverage decreased (-0.007%) to 77.485%

Changes Missing Coverage Covered Lines Changed/Added Lines %
src/tpm2/NVMarshal.c 8 10 80.0%
src/tpm2/Unmarshal.c 9 14 64.29%
Files with Coverage Reduction New Missed Lines %
src/tpm2/Unmarshal.c 1 89.03%
Totals Coverage Status
Change from base Build 2045: -0.007%
Covered Lines: 29063
Relevant Lines: 37508

💛 - Coveralls

@stefanberger stefanberger changed the title Reset buffer size indicators that are too large and check for maximu size on marshalling Reset buffer size indicators that are too large and check for maximum size on marshalling Jun 22, 2021
@stefanberger stefanberger merged commit 7981d9a into master Jun 23, 2021
3 of 4 checks passed
@elmarco
Copy link
Contributor

elmarco commented Jun 23, 2021

Isn't there anything better than assert() that could be done? If it can be triggered from the guest, it may still be exploitable to break the VM in some ways.

@stefanberger
Copy link
Owner Author

@elmarco What do you suggest?

@elmarco
Copy link
Contributor

elmarco commented Jun 23, 2021

If the assert can't be triggered at runtime (which you told me it shouldn't), it is fine.

@stefanberger
Copy link
Owner Author

Yes, that would be a violation of the buffer size...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants