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

bughuntoor - Usage of tx.origin to determine the user is prone to attacks #82

Open
sherlock-admin2 opened this issue Sep 9, 2024 · 6 comments
Labels
Escalation Resolved This issue's escalations have been approved/rejected Has Duplicates A valid issue with 1+ other issues describing the same vulnerability Medium A Medium severity issue. Reward A payout will be made for this issue Sponsor Disputed The sponsor disputed this issue's validity

Comments

@sherlock-admin2
Copy link
Contributor

sherlock-admin2 commented Sep 9, 2024

bughuntoor

High

Usage of tx.origin to determine the user is prone to attacks

Summary

Usage of tx.origin to determine the user is prone to attacks

Vulnerability Detail

Within core.vy to user on whose behalf it is called is fetched by using tx.origin.

  self._INTERNAL()

  user        : address   = tx.origin

This is dangerous, as any time a user calls/ interacts with an unverified contract, or a contract which can change implementation, they're put under risk, as the contract can make a call to api.vy and act on user's behalf.

Usage of tx.origin would also break compatibility with Account Abstract wallets.

Impact

Any time a user calls any contract on the BOB chain, they risk getting their funds lost.
Incompatible with AA wallets.

Code Snippet

https://github.com/sherlock-audit/2024-08-velar-artha/blob/main/gl-sherlock/contracts/core.vy#L166

Tool used

Manual Review

Recommendation

Instead of using tx.origin in core.vy, simply pass msg.sender as a parameter from api.vy

@github-actions github-actions bot added Has Duplicates A valid issue with 1+ other issues describing the same vulnerability Medium A Medium severity issue. labels Sep 11, 2024
@sherlock-admin3 sherlock-admin3 changed the title Kind Banana Sloth - Usage of tx.origin to determine the user is prone to attacks bughuntoor - Usage of tx.origin to determine the user is prone to attacks Sep 11, 2024
@sherlock-admin3 sherlock-admin3 added Reward A payout will be made for this issue Sponsor Disputed The sponsor disputed this issue's validity labels Sep 11, 2024
@T1MOH593
Copy link

Escalate

Noticed there were 19 escalations on preliminary valid issues. This is final escalation to make it 20/20 🙂

@sherlock-admin3
Copy link
Contributor

Escalate

Noticed there were 19 escalations on preliminary valid issues. This is final escalation to make it 20/20 🙂

You've created a valid escalation!

To remove the escalation from consideration: Delete your comment.

You may delete or edit your escalation comment anytime before the 48-hour escalation window closes. After that, the escalation becomes final.

@sherlock-admin4 sherlock-admin4 added the Escalated This issue contains a pending escalation label Sep 12, 2024
@WangSecurity

This comment was marked as off-topic.

@WangSecurity
Copy link

Planning to reject the escalation and leave the issue as it is.

@WangSecurity
Copy link

Result:
Medium
Has duplicates

@sherlock-admin2 sherlock-admin2 removed the Escalated This issue contains a pending escalation label Sep 19, 2024
@sherlock-admin3 sherlock-admin3 added the Escalation Resolved This issue's escalations have been approved/rejected label Sep 19, 2024
@sherlock-admin4
Copy link
Contributor

Escalations have been resolved successfully!

Escalation status:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Escalation Resolved This issue's escalations have been approved/rejected Has Duplicates A valid issue with 1+ other issues describing the same vulnerability Medium A Medium severity issue. Reward A payout will be made for this issue Sponsor Disputed The sponsor disputed this issue's validity
Projects
None yet
Development

No branches or pull requests

5 participants