search

sherlock-audit / 2024-01-rio-vesting-escrow-judging

3 stars 2 forks source link

OrderSol - Owner has excessive access to user funds #91

Closed sherlock-admin2 closed 9 months ago

sherlock-admin2 commented 9 months ago

OrderSol

high

Owner has excessive access to user funds

Summary

As user must approve VestingEscrowFactory for token, owner can get access to any funds thus approved.

Vulnerability Detail

Steps: 1) user approves VestingEscrowFactory for X tokens, where X often is uint256.max due to convenince or UX. 2) owner then can frontrun user with recoverERC20 before user calls deployVestingContract, or maliciously or erroneously move funds after.

Impact

HIGH - user funds at risk.

Code Snippet

https://github.com/sherlock-audit/2024-01-rio-vesting-escrow/blob/main/rio-vesting-escrow/src/VestingEscrowFactory.sol#L80

Tool used

Manual Review

Recommendation

Make recoverERC20 allow to move funds only if token_ != token (not the token set up in constructor)

nevillehuang commented 9 months ago

Invalid, admins are trusted entities trusted to not be malicious. See point 5. of sherlock rules. Additionally, the factory contract is purely used for deployment of escrow contracts only

sherlock-admin2 commented 9 months ago

1 comment(s) were left on this issue during the judging contest.

pratraut commented:

'invalid due to owner being TRUSTED entity'