Check input in rustls_error to avoid UB #195
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The input parameter for rustls_error was `rustls_result`. However, in Rust it's undefined behavior for an enum to hold an invalid value. That meant that if C passed an invalid value to rustls_error, UB would result. This changes the input parameter to be a uint, and relies on a macro from the num_enum crate to check the value of that input parameter. If the input is invalid, we emit the error for "InvalidParameter".
djc
reviewed
Nov 8, 2021
| @@ -14,6 +14,7 @@ libc = "0.2" | |||
| sct = "0.7" | |||
| rustls-pemfile = "0.2.1" | |||
| log = "0.4.14" | |||
| num_enum = "0.5.4" | |||
There was a problem hiding this comment.
IMO the extra dependency for this is overkill. In this case it seems fine to write a custom TryFrom impl based on the repr type.
|
I was reluctant to take the extra dep, too. The problem is, the list of
error codes is quite long, and I want to programmatically guarantee that
the TryFrom impl is complete (covers all the numbers) and accurate (maps to
the correct enum variant).
When I think of ways to do that in Rust, they mostly wind up looking like
proc macros. And somebody has already written the appropriate proc macro.
I'm interested in other ways to achieve the same goal, though. Do you have
a way in mind to ensure completeness and accuracy here?
|
|
Yeah, it's not exactly trivial. Maybe something like the enum builder declarative macro we use in rustls? |
|
I'm going to merge as-is, then file a separate issue for achieving this without a dependency. |
|
Follow-up issue: #198 |
This was referenced Nov 9, 2021
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The input parameter for rustls_error was
rustls_result. However, inRust it's undefined behavior for an enum to hold an invalid value. That
meant that if C passed an invalid value to rustls_error, UB would
result.
This changes the input parameter to be a uint, and relies on a macro
from the num_enum crate to check the value of that input parameter. If
the input is invalid, we emit the error for "InvalidParameter".
Part of #152.