VTVL contest - 0xdapper's results

Lines of code

https://github.com/code-423n4/2022-09-vtvl/blob/main/contracts/VTVLVesting.sol#L428-L434 https://github.com/code-423n4/2022-09-vtvl/blob/main/contracts/VTVLVesting.sol#L105-L112 https://github.com/code-423n4/2022-09-vtvl/blob/main/contracts/VTVLVesting.sol#L364-L365

Vulnerability details

Impact

Seems like it is intended to have this feature where admins are able to revoke user claims as they fit and in doing so they revoke all the user's already vested but not withdrawn amount as well.

However, this in itself is very frontrunnable by the users themselves, if they see the revocation tx in the mempool from the admin, they can frontrun that transaction to withdraw their vested amounts before the revocation transaction gets mined by bumping gas prices on their transaction for faster inclusion.

So, it is a false assurity that the founders can revoke unwithdrawn but vested amounts too by revoking an employee's claim.

Proof of Concept

https://github.com/code-423n4/2022-09-vtvl/blob/main/contracts/VTVLVesting.sol#L428-L434 -- the revoke function where the claim.isActive is set to false without transferring any vested but not withdrawn amount to the user because thats the feature https://github.com/code-423n4/2022-09-vtvl/blob/main/contracts/VTVLVesting.sol#L105-L112 -- the modifier where the claim is checked for being active https://github.com/code-423n4/2022-09-vtvl/blob/main/contracts/VTVLVesting.sol#L364-L365 -- withdraw function where it is first checked that the claim is still active i.e. claim.isActive == true

Tools Used

Recommended Mitigation Steps

If this feature is important to the founders for early termination or other reasons founders should be using larger cliffs. Since at the moment in the current state of code the founders can always be frontran might as well make it a feature and explicitly transfer vested unwithdrawn amounts to the users for their claims and institute good faith and trust with the parties you are vesting tokens to.