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

3 stars 2 forks source link

itsabinashb - VestingEscrow::Unauthorised voting #64

Closed sherlock-admin closed 9 months ago

sherlock-admin commented 9 months ago

itsabinashb

medium

VestingEscrow::Unauthorised voting

Summary

After ending a vesting period the recipient can claim tokens, the current logic considers users as recipient even after claiming their tokens. As a result they can vote. Although the voting power is not incremented if the user claim all of tokens.

Vulnerability Detail

The claim() of VestingEscrow.sol contract allows an recipient to claim tokens after vesting period ends. But there is no logic which invalidate the user after claiming the token. As a result he can vote even after long time of claiming. Actually it is not profitable for the user if he claims all tokens, but if he claims some amount of token and then vote then he can potentially increase the voting power of the delegatee, it should not be. We can verify this with test, just comment out this and this line of TestUtil.sol contract, create a test file in test folder and paste this test case in that file:

// SPDX-License-Identifier: MIT
pragma solidity 0.8.23;

import {TestUtil} from 'test/lib/TestUtil.sol';
import {ERC20Token} from 'test/lib/ERC20Token.sol';
import 'forge-std/console.sol';

contract Audit is TestUtil {
  address owner = vm.addr(1);
  address manager = vm.addr(2);
  address user1 = vm.addr(3);
  address delegatee = vm.addr(9);

  function setUp() external {
    setUpProtocol(ProtocolConfig(owner, manager));
    console.log(owner);
  }
function test_alwaysRecipient() external {
    vm.startPrank(user1);
    deployVestingEscrow(VestingEscrowConfig(10 ether, user1, 1 days, uint40(block.timestamp), 20 hours, true, ''));
    vm.warp(2 days + block.timestamp);

   // Claiming all tokens

    deployedVesting.claim(user1, 10 ether);
    console.log("user's balance after claiming:", token.balanceOf(user1));
    vm.warp(10 days);

   // 10 days passed after claiming

    deployedVesting.delegate(ozVotingAdaptor.encodeDelegateCallData(delegatee));
    uint256 proposalId = createProposal();
    vm.roll(block.number + 1);

    // Voting

    deployedVesting.vote(ozVotingAdaptor.encodeVoteCallData(proposalId, uint8(VoteType.For)));
    console.log("voting power of deleatee:", token.getVotes(delegatee));
    vm.stopPrank();
  }
}

The result:

[PASS] test_alwaysRecipient() (gas: 552096)
Logs:
  0x7E5F4552091A69125d5DfCb7b8C2659029395Bdf
  user's balance after claiming: 10000000000000000000
  voting power of deleatee: 0

Test result: ok. 1 passed; 0 failed; 0 skipped; finished in 2.13ms

Here we can see if the recipient withdraws all tokens then he can't increase the voting power, but he can claim minimum amount of token and use rest of tokens to increase the voting power of delegatee, replace the above test function with this test function:

function test_alwaysRecipient() external {
    vm.startPrank(user1);
    deployVestingEscrow(VestingEscrowConfig(10 ether, user1, 1 days, uint40(block.timestamp), 20 hours, true, ''));

    vm.warp(2 days + block.timestamp);
   // Claiming 1 ether
    deployedVesting.claim(user1, 1 ether);
    console.log("user's balance after claiming:", token.balanceOf(user1));
    vm.warp(10 days);
   // 10 days passed after claiming
    deployedVesting.delegate(ozVotingAdaptor.encodeDelegateCallData(delegatee));

    uint256 proposalId = createProposal();
    vm.roll(block.number + 1);
    // Voting
    deployedVesting.vote(ozVotingAdaptor.encodeVoteCallData(proposalId, uint8(VoteType.For)));
    console.log("voting power of deleatee:", token.getVotes(delegatee));
    vm.stopPrank();
  }

Here the recipient claimed 1 ether. Result of this test is:

[PASS] test_alwaysRecipient() (gas: 617525)
Logs:
  0x7E5F4552091A69125d5DfCb7b8C2659029395Bdf
  user's balance after claiming: 1000000000000000000
  voting power of deleatee: 9000000000000000000

Test result: ok. 1 passed; 0 failed; 0 skipped; finished in 2.45ms

You can see how the user increased the voting power of the delegatee.

Impact

User can vote even after claiming tokens.

Code Snippet

  1. https://github.com/sherlock-audit/2024-01-rio-vesting-escrow/blob/main/rio-vesting-escrow/src/VestingEscrow.sol#L136

    Tool used

Manual Review, Foundry

Recommendation

Invalidate the user so that he can't vote after claiming.

nevillehuang commented 9 months ago

Similar to #57, once recipient claim tokens, they are free to delegate and allow delegatees to vote, no issue here. In your second test, recipieint user 1 claims 1 ether, so contract has 9 ether of voting power left so delegatee should also have 9 ether of voting power, no issue here