sherlock-admin2
commented
9 months ago 1 comment(s) were left on this issue during the judging contest.
_rahul commented:
Invalid: Recipient has authorized escrow to be fully revokable to help recover funds (incase of loss of recipient address etc) during setup. Essentially, owner calls revokeAll() to rescue funds for the recipient. In this context, it’s unlikely that will recipient front-run revokeAll().
jasonxiale
medium
VestingEscrow.revokeAllcan be front-run byVestingEscrow.claimSummary
In VestingEscrow.claim, when there are unclaimed token available the recipient can choose which address to receive the token by beneficiary parameter, it means the token can still under recipient's possess. But in VestingEscrow.revokeAll, the unclaimed will be sent to the owner together with locked token, which means the unclaimed token will under owner's possess. So if there are some unclaimed token in the contract, the recipient can front-run the factory owner to claim the extra token
Vulnerability Detail
In VestingEscrow.claim, when there are unclaimed token available the recipient can choose which address to receive the token by beneficiary parameter, it means the token can still under recipient's possess. But in VestingEscrow.revokeAll, the unclaimed will be sent to the owner together with locked token, which means the unclaimed token will under owner's possess. So if there are some unclaimed token in the contract, the recipient can front-run the factory owner to claim the extra token
Impact
VestingEscrow.revokeAllcan be front-run byVestingEscrow.claimCode Snippet
https://github.com/sherlock-audit/2024-01-rio-vesting-escrow/blob/main/rio-vesting-escrow/src/VestingEscrow.sol#L136-L144 https://github.com/sherlock-audit/2024-01-rio-vesting-escrow/blob/main/rio-vesting-escrow/src/VestingEscrow.sol#L176-L189
Tool used
Manual Review
Recommendation
Duplicate of #63