zzykxx - Calls to `revokeAll()` in `VestingEscrow.sol` can be frontrun

[sherlock-admin2]

zzykxx

medium

Calls to revokeAll() in VestingEscrow.sol can be frontrun

Summary

Recipients can frontrun calls to revokeAll() with a call to claim(), claiming tokens they should not be able to receive.

Vulnerability Detail

The revokeAll() function in VestingEscrow.sol can be called (by the owner) to revoke all tokens from the recipient, including already vested tokens that could be withdrawn.

A sophisticated recipient could monitor the mempool for calls to revokeAll() and frontrun the transactions by calling the function claim() before the call to revokeAll() is executed.

Impact

Recipients are able to retrieve tokens they should not be able to get, since a call to revokeAll() implies all tokens, including currently vested ones, should be revoked.

Code Snippet

• revokeAll()
• claim()

  Tool used

Manual Review

Recommendation

Other protocols solved this issue by implementing a withdrawal queue. Meaning the recipient signals his intention to withdraw the vested tokens and only after a pre-deteremined amount of time (ex. 3 days) they are able to transfer the vested tokens.

Duplicate of #63

[sherlock-admin2]

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().