PoolTogether - 0xkasper's results

Lines of code

https://github.com/GenerationSoftware/pt-v5-vault/blob/b1deb5d494c25f885c34c83f014c8a855c5e2749/src/Vault.sol#L982-L994

Vulnerability details

Impact

The sponsor function in the Vault.sol contract allows anyone to remove another user's delegation by forcing them to delegate to the sponsor address. _sponsor will deposit some amount from the caller for the target user and then force a delegation to the sponsor address (address(1)).

However, this amount can just be 0 and so it becomes a function to simply force a removal of a delegation. The full delegated power gets removed, because delegations to the sponsor address are not tracked.

As such, it becomes possible to call sponsor for every user and make the total delegated power supply in the TwabController equal to 0. The attacker can then be the only one with some delegated amount that is equal to 100% of the total supply, manipulating the process of the lottery.

Rectifying the delegation requires manual interaction from the user and the exploit can be repeated anytime, continuously, further manipulating the values in the TwabController.

Proof of Concept

function testPoCDelegateRemoval() public {
    address SPONSORSHIP_ADDRESS = address(1);
    uint96 BALANCE = 100_000_000 ether;

    token.approve(address(vault), BALANCE);
    vault.deposit(BALANCE, address(this));

    assertEq(address(this), twab_controller.delegateOf(address(vault), address(this)));
    assertEq(BALANCE, twab_controller.delegateBalanceOf(address(vault), address(this)));

    // As attacker, call sponsor with 0 amount and victim address
    vm.prank(address(0xdeadbeef));
    vault.sponsor(0, address(this));

    // Delegated balance is now gone
    assertEq(SPONSORSHIP_ADDRESS, twab_controller.delegateOf(address(vault), address(this)));
    assertEq(0, twab_controller.delegateBalanceOf(address(vault), address(this)));
    assertEq(0, twab_controller.delegateBalanceOf(address(vault), SPONSORSHIP_ADDRESS));
}

Tools Used

Manual review, VSCode, Foundry.

Recommended Mitigation Steps

The sponsor function should only accept deposits if the receiver has already delegated to the sponsorship address. Or otherwise, the deposit is accepted, but the delegation should not be forced.

Assessed type

Other

Lines of code

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#L590-L603

Vulnerability details

Impact

The function TwabController.sol:_delegateOf will return the address of the user if the delegation is set to address(0). This makes it the default that users will automatically gain delegated power upon minting or transfers.

However, it leads to a bug if the user delegates to address(0):

• On line 649 it would return the user as the current delegate.
• The new delegate will stay address(0).
• In _transferDelegateBalance, the user gets their delegated power subtracted and addition to the address(0) is skipped.

Due to the nature of _delegateOf, the user is still delegated to themselves in any subsequent calls to transfer, burn or delegate, but the delegated balance is gone. Delegating to address(0) can happen to any user that wants to for example remove a delegation or by accident (since it is the default value). It would result in the assets of the user to get permanently locked, as trying to transfer, re-delegate or burn would revert the transaction due to underflow. Withdawals or redeems from the vault will also revert, because this calls TwabController.burn().

Proof of Concept

The vulnerability can be reproduced by simply delegating any vault to address(0). Any subsequent delegate, transfer or burn will revert.

The follow code shows the vulnerability:

function testPoC() public {
    address VAULT = address(0xdeadbeef);
    uint96 BALANCE = 100 ether;

    vm.prank(VAULT);
    twab_controller.mint(address(this), BALANCE);
    assertEq(BALANCE, twab_controller.balanceOf(VAULT, address(this)));

    twab_controller.delegate(VAULT, address(0));

    // Cannot delegate to other anymore
    vm.expectRevert();
    twab_controller.delegate(VAULT, address(1337));

    // Cannot delegate to oneself anymore
    vm.expectRevert();
    twab_controller.delegate(VAULT, address(this));

    // Cannot transfer the balance anymore
    vm.expectRevert();
    vm.prank(VAULT);
    twab_controller.transfer(address(this), address(1337), BALANCE);

    // Cannot burn the balance anymore
    vm.expectRevert();
    vm.prank(VAULT);
    twab_controller.burn(address(this), BALANCE);
}

Tools Used

Manual, VSCode, Foundry

Recommended Mitigation Steps

The function _delegate should take the special case of address(0) into account.

Assessed type

Other