Front-Running Vulnerability in revokeAll Function of VestingEscrow Contract

Summary

A potential front-running vulnerability exists in the revokeAll function of the VestingEscrow contract. This issue allows a recipient to potentially claim unvested tokens before the owner's revocation transaction is processed, leading to an unexpected depletion of the tokens intended to be revoked.

Vulnerability Detail

The revokeAll function is designed to allow the contract owner to revoke all tokens (locked and unclaimed) from the contract. However, this function is susceptible to a front-running attack. A malicious or opportunistic recipient can monitor the Ethereum mempool for revokeAll transactions and execute a claim transaction with a higher gas fee to ensure it is processed first. This action allows the recipient to withdraw the remaining portion of the unclaimed tokens, reducing the amount available for revocation when the revokeAll transaction is subsequently processed.

Impact

It undermines the intended functionality of the revokeAll feature and can result in a lesser amount of tokens being revoked than anticipated by the owner. This could impact the contract's financial and operational aspects, particularly in scenarios where precise control over token vesting and revocation is crucial.

Code Snippet

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

Introducing a claimRequest function with a time delay before tokens can actually be claimed is a viable strategy to mitigate the front-running issue in your smart contract. This approach adds a layer of protection by separating the request to claim tokens from the actual transfer of tokens. Here's an overview of how this could work and its implications:

Claim Request Function (claimRequest):

Claim Request Function (claimRequest):
- This function allows the recipient to initiate a claim. It records the request along with a timestamp.
- It could set a state variable indicating that a claim has been requested and store the amount and the time when the request was made.
Claim Execution Function (executeClaim):
- After the time delay has passed since the claimRequest, the recipient can call this function to actually transfer the tokens.
- The function checks if the necessary time has elapsed since the claimRequest.

mapping(address => uint256) public claimRequestTimestamps;
mapping(address => uint256) public requestedAmounts;
uint256 public claimDelay = 1 days;

function claimRequest(uint256 amount) external onlyRecipient {
    require(block.timestamp >= claimRequestTimestamps[msg.sender] + claimDelay, "Previous request still pending");
    claimRequestTimestamps[msg.sender] = block.timestamp;
    requestedAmounts[msg.sender] = amount;
    // Additional logic if needed
}

function executeClaim() external onlyRecipient {
    require(block.timestamp >= claimRequestTimestamps[msg.sender] + claimDelay, "Claim delay not yet passed");
    uint256 amount = requestedAmounts[msg.sender];
    // Execute the claim logic, transfer the tokens, etc.
    // Reset or update the request state
}

This approach minimizes the risk of front-running since the recipient cannot immediately claim the tokens till (block.timestamp >= claimRequestTimestamps[msg.sender] + claimDelay) == true.

Duplicate of #63

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

0xmystery - Front-Running Vulnerability in revokeAll Function of VestingEscrow Contract #103