Make it harder to forge Throw tokens
#797
Merged
Conversation
This file contains 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
kjvalencik
reviewed
Sep 20, 2021
- added a private field to the type - changed the test case that forges a `Throw` token to be much more devious (since it's so much harder to forge now)
dherman
force-pushed
the
unforgeable-throw
branch
from
2ee0ac6
to
9c054af
Compare
- use private phantom !Send+!Sync data to make Throw even slightly more secure - use a crate-private constructor method to clean up construction code throughout the crate - simpler logic for forge test
kjvalencik
approved these changes
Nov 17, 2021
…hrow` harder to store
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.
This PR makes it harder (though not technically impossible) to forge
Throwtokens. We already have safety mechanisms in place in case of forgedThrowtokens, leading to well-defined panics (as opposed to something unsafe, like undefined behavior). But this should help prevent programs from accidentally saving and reusing aThrowtoken.There is also a test case that still does show how it's technically possible to forge a token, and demonstrates that it still has predictable behavior.
An alternative that would be even safer would be to place a lifetime on a
Throwtoken, making it impossible to store them in long-lived storage. But this would leak into the type signatures of e.g.NeonResultand APIs that use it, which would add a lot of complexity to Neon for little value. The panic is for a rare enough case that the extra type safety wouldn't be worth it IMO.One more change I want to make in this PR is to add
!Sendphantom data to theThrowtype so that it can't be shared outside of its thread.