-
Notifications
You must be signed in to change notification settings - Fork 18
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
All tokens can be stolen from VirtualAccount
due to missing access modifier
#885
Comments
0xA5DF marked the issue as duplicate of #888 |
0xA5DF marked the issue as sufficient quality report |
0xA5DF marked the issue as high quality report |
alcueca marked the issue as satisfactory |
alcueca marked the issue as selected for report |
alcueca marked the issue as not selected for report |
alcueca marked the issue as selected for report |
Added the label "primary issue" as it was missing here |
0xLightt (sponsor) confirmed |
Issue addressed at Maia-DAO/2023-09-maia-remediations@2209d6e |
Lines of code
https://github.com/code-423n4/2023-09-maia/blob/f5ba4de628836b2a29f9b5fff59499690008c463/src/VirtualAccount.sol#L85
Vulnerability details
Impact
All non-native assets (ERC20 tokens, NFTs, etc.) can be stolen by anyone from a
VirtualAccount
using its payableCall(...) method, which lacks the necessary access control modifier requiresApprovedCaller. See also, the call(...) method which utilizes the requiresApprovedCaller modifier.Therefore, an attacker can craft a call to e.g.
ERC20.transfer(...)
on behalf of the contract, like the withdrawERC20(...) method does, while bypassing access control by executing the call via payableCall(...).As a consequence, all non-native assets of the
VirtualAccount
can be stolen by anyone causing a loss for its owner.Proof of Concept
Add the code below as new test file
test/ulysses-omnichain/VirtualAccount.t.sol
and run it usingforge test -vv --match-contract VirtualAccountTest
in order to verify the above claims.Output:
Tools Used
Manual review
Recommended Mitigation Steps
Add the missing requiresApprovedCaller modifier to the payableCall(...) method:
Assessed type
Access Control
The text was updated successfully, but these errors were encountered: