VotingEscrow::Unexpected behaviour in voting power

Summary

An user can delegate more than 1 delegatee and increase the voting power each of them, this increment is malicious because the total voting power will be far more than that user's locked amount.

Vulnerability Detail

The system allows a user to delegate more than 1 delegatee. That user can partially claim his amount and use those amount to fund the escrow contract and delegate a new delegatee as many times as he wants until his token balance becomes 0. We can see it in test case. To conduct the test we will have to modify the TestUtil.sol file, just comment out this and this line, make a test file inside test folder and paste this test case:


// 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);
  address delegatee2 = vm.addr(10);
  address delegatee3 = vm.addr(11);

  function setUp() external {
    setUpProtocol(ProtocolConfig(owner, manager));
  }
function test_multipleDelegation() external {
    vm.startPrank(user1);
    deployVestingEscrow(VestingEscrowConfig(3 ether, user1, 10 days, uint40(block.timestamp), 5 days, true, ''));
    vm.warp(10 days + block.timestamp);

    // Claiming 2 ether, so 1 ether will be used to provide voting power to delegatee
    deployedVesting.claim(user1, 2 ether);
    console.log("after claiming user1's balance:", token.balanceOf(user1));
    console.log("balance of escrow contract after 1st claim:", token.balanceOf(address(deployedVesting)));

    // Delegated delegatee
    deployedVesting.delegate(ozVotingAdaptor.encodeDelegateCallData(delegatee));
    console.log("Delegated first time");
    console.log("voting power of delegatee:", token.getVotes(delegatee));

    //transfering 1 ether to escrow contract as the escrow contract funds the delegatee
    token.transfer(address(deployedVesting), 1 ether);
    console.log("after transfering to escrow contract the balance of the user1 is:", token.balanceOf(user1));
    console.log("balance of escrow contract after 1st transfer:", token.balanceOf(address(deployedVesting)));

    // escrow contract successfully funded the delegatee2 
    deployedVesting.delegate(ozVotingAdaptor.encodeDelegateCallData(delegatee2));
    console.log("after delegating 2nd time the balance of the user1 is:", token.balanceOf(user1));
    console.log("voting power of delegatee2:", token.getVotes(delegatee2));
    console.log("delegated 2nd time");

    // again transfering 1 ether to escrow contract, purpose is same, funding the escrow contract so that it can fund delegatee3
    token.transfer(address(deployedVesting), 1 ether);
    console.log("after transfering again to escrow contract the balance of the user1 is:", token.balanceOf(user1));
    console.log("balance of escrow contract after 1st transfer:", token.balanceOf(address(deployedVesting)));

    // escrow contract successfully funded delegatee3
    deployedVesting.delegate(ozVotingAdaptor.encodeDelegateCallData(delegatee3));
    console.log("voting power of delegatee3:", token.getVotes(delegatee3));
    console.log("Delegated 3rd time");

  }
}

The result is:

[PASS] test_multipleDelegation() (gas: 647078)
Logs:
  after claiming user1's balance: 2000000000000000000
  balance of escrow contract after 1st claim: 1000000000000000000
  Delegated first time
  voting power of delegatee: 1000000000000000000
  after transfering to escrow contract the balance of the user1 is: 1000000000000000000
  balance of escrow contract after 1st transfer: 2000000000000000000
  after delegating 2nd time the balance of the user1 is: 1000000000000000000
  voting power of delegatee2: 2000000000000000000
  delegated 2nd time
  after transfering again to escrow contract the balance of the user1 is: 0
  balance of escrow contract after 1st transfer: 3000000000000000000
  voting power of delegatee3: 3000000000000000000
  Delegated 3rd time

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

Here we can clearly see that total voting power provided by user1 is (1000000000000000000+2000000000000000000+3000000000000000000) = 6000000000000000000 i.e 6 ether but he vested only 3 ether.

Impact

An user can maliciously delegate multiple delegatee and increase total voting power far more than his locked amount.

Code Snippet

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

Manual Review, Foundry

Recommendation

Add some logic which implements a restriction on multiple delegation.

nevillehuang commented 9 months ago

Invalid, in the test the recipient user1 is delegating to 3 separate delegatees based on token blanace in vesting contract so voting power is reflected correctly.

sherlock-admin2 commented 9 months ago

2 comment(s) were left on this issue during the judging contest.

_rahul commented:

Watson misunderstood how delegation works.

pratraut commented:

'invalid due to owner who is deploying escrow contract is TRUSTED entity'

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

itsabinashb - VotingEscrow::Unexpected behaviour in voting power #88