-
Notifications
You must be signed in to change notification settings - Fork 166
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
fix(torii): offchain messages signature validation on first set #2390
fix(torii): offchain messages signature validation on first set #2390
Conversation
WalkthroughOhayo, sensei! The changes involve a restructuring of the identity handling logic within the Changes
Sequence Diagram(s)sequenceDiagram
participant Relay
participant Message
participant Identity
Relay->>Message: Check entity_identity
alt entity_identity is Some(identity)
Relay->>Identity: Parse identity
alt Parsing successful
Relay->>Relay: Set entity
else Parsing failed
Relay->>Relay: Log warning
end
else entity_identity is None
Relay->>Message: Retrieve identity from ty
Relay->>Identity: Validate identity
alt Validation successful
Relay->>Relay: Set entity
else Validation failed
Relay->>Relay: Return error
end
end
Recent review detailsConfiguration used: .coderabbit.yaml Files selected for processing (1)
Files skipped from review as they are similar to previous changes (1)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@edisontim thanks a lot for tackling that man, as we've discussed, it was a potential security issue.
Some comments with few changes, and let's merge. :)
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2390 +/- ##
==========================================
+ Coverage 68.30% 68.37% +0.07%
==========================================
Files 357 357
Lines 47181 47188 +7
==========================================
+ Hits 32225 32267 +42
+ Misses 14956 14921 -35 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the update @edisontim!
Description
When an offchain message is sent to Torii and in the case the entity isn't present in the DB yet, the signature of the message isn't checked.
This assumes that the signature is valid for that address which might not be the case if someone is using an account address that doesn't match the private key he used to sign the message.
It could present some risks in some cases: in haiku we use a private key linked to an address when sending prompts to Torii. The keys of these offchain messages change for every prompt so their signature will never be checked. In that case, anyone could send any message that would be interpreted by our frontend as a prompt signed by us and display it to the user.
Related issue
N/A
Summary by CodeRabbit
New Features
Bug Fixes