code423n4 commented 1 year ago

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

c4-judge commented 1 year ago

Picodes marked the issue as duplicate of #293