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

Responder can Invoke Undefined Behavior in libspdm Requester #2068

Closed
steven-bellock opened this issue May 29, 2023 · 6 comments · Fixed by #2069
Closed

Responder can Invoke Undefined Behavior in libspdm Requester #2068

steven-bellock opened this issue May 29, 2023 · 6 comments · Fixed by #2069
Assignees
Labels
bug Something isn't working security An issue that impacts security
Milestone

Comments

@steven-bellock
Copy link
Contributor

Following a successful CAPABILITIES response a libspdm Requester stores the Responder's CTExponent into its context without validation. If the Requester sends a request message that requires a cryptography operation by the Responder, such as CHALLENGE, libspdm will calculate the timeout value using the Responder's unvalidated CTExponent.

((uint64_t)2 << context->connection_info.capability.ct_exponent)

If the value of CTExponent is greater than or equal to 64 then C language undefined behavior is invoked.
Note that libspdm provides a method to query the value of the CTExponent after VCA.

This also happens with RDTExponent. Note also that shifting the value 2 is incorrect. It should be 1.

@steven-bellock steven-bellock added bug Something isn't working security An issue that impacts security labels May 29, 2023
@steven-bellock steven-bellock added this to the Q2 2023 milestone May 29, 2023
@steven-bellock steven-bellock self-assigned this May 29, 2023
steven-bellock added a commit to steven-bellock/libspdm that referenced this issue May 29, 2023
Fix DMTF#2068 in main branch.

Signed-off-by: Steven Bellock <sbellock@nvidia.com>
steven-bellock added a commit to steven-bellock/libspdm that referenced this issue May 29, 2023
Fix DMTF#2068 in 2.3 branch.

Signed-off-by: Steven Bellock <sbellock@nvidia.com>
jyao1 pushed a commit that referenced this issue May 30, 2023
Fix #2068 in main branch.

Signed-off-by: Steven Bellock <sbellock@nvidia.com>
jyao1 pushed a commit that referenced this issue May 30, 2023
Fix #2068 in 2.3 branch.

Signed-off-by: Steven Bellock <sbellock@nvidia.com>
@wmaroneAMD
Copy link
Contributor

The spec does not specify a limit beyond the maximum value of a single byte. What was the reasoning for limiting it to <= 2^31?

@steven-bellock
Copy link
Contributor Author

It seems like a reasonable maximum value. If a Responder takes 35 minutes to process a response does any Requester really want to talk to it?

@wmaroneAMD
Copy link
Contributor

Entirely fair. This does seem like a diversion from the spec as a result of a gap in the spec. In addition to this upper-bound here, could we move to have this upper bound defined in the spec for future versions of SPDM?

@steven-bellock
Copy link
Contributor Author

I filed https://github.com/DMTF/SPDM-WG/issues/2799 but it was not approved for SPDM 1.x. So that would get fixed in SPDM 2.0, whenever that happens.

@wmaroneAMD
Copy link
Contributor

@steven-bellock thank you for doing that, it's minor but it makes the spec better. Would you be OK adding that link in a comment near those defines, to provide guidance in case someone else comes across this?

@steven-bellock
Copy link
Contributor Author

Sure.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working security An issue that impacts security
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants