Attacker can force users to delegate to SPONSORSHIP_ADDRESS

[code423n4]

Lines of code

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

Vulnerability details

Impact

An attacker can change the delegatee of a user who deposited into the vault to the SPONSORSHIP_ADDRESS address by calling one of the functions sponsor or sponsorWithPermit and giving the address of the user as _receiver.

The impact of this issue is that the attacker can stop users from winning prizes because the users who have delegated to the SPONSORSHIP_ADDRESS address get their chances of winning prizes revoked.

Proof of Concept

As the functions sponsor and sponsorWithPermit do the same thing except the later include a permit functionality which doesn't have an impact on this issue, i will explain how this issue can be exploited with the sponsor function and the same apply to the sponsorWithPermit function.

We start with the sponsor function :

function sponsor(uint256 _assets, address _receiver) external returns (uint256) {
    return _sponsor(_assets, _receiver);
}

As you can see the function can be called by anyone, and it takes the _assets amount of asset to deposit into the vault and a _receiver address that the user wants to deposit to, the function calls the internal function _sponsor below :

function _sponsor(uint256 _assets, address _receiver) internal returns (uint256) {
    uint256 _shares = deposit(_assets, _receiver);

    if (
      _twabController.delegateOf(address(this), _receiver) != _twabController.SPONSORSHIP_ADDRESS()
    ) {
      _twabController.sponsor(_receiver);
    }
    
    emit Sponsor(msg.sender, _receiver, _assets, _shares);
    
    return _shares;
}

Inside this function we can see that it call the deposit function for performing the deposit of the asset amount, and then the function checks the current delegatee of the _receiver address, if this one is different from the SPONSORSHIP_ADDRESS address the function will call the _twabController.sponsor function to changes the _receiver delegatee address to the SPONSORSHIP_ADDRESS address :

// TwabController.sponsor
function sponsor(address _from) external {
    _delegate(msg.sender, _from, SPONSORSHIP_ADDRESS);
}

In the comments of the TwabController contract we can see the following :

/// @notice Allows users to revoke their chances to win by delegating to the sponsorship address.
address public constant SPONSORSHIP_ADDRESS = address(1);

Basically any users who is delegating to the SPONSORSHIP_ADDRESS address is not included in the prize distribution and has no chances to win.

So to resume any attacker can change all other users delegations to the SPONSORSHIP_ADDRESS address and deprive them from the chance of winning prizes, and this attack will not cost the attacker any funds as he can at each time deposit a very small amount to every user when calling the sponsor function.

The scenario of the attack can be summarized as follows :

• Alice has deposited funds into a given Vault, and she has not delegated them so in the TwabController contract she is basically delegating to her self meaning : delegates[_vault][Alice] = Alice.

• The attacker Bob sees that Alice has deposited funds into the vault, so he calls the sponsor function with the following parameters : _assets = 10 (choose 10 just for the example but could be any other very small amount) and _receiver = Alice.

• The sponsor function after calling deposit to deposit _assets = 10 to Alice, proceeds to check the current delegatee of Alice and it finds that it is different from the SPONSORSHIP_ADDRESS address : delegates[_vault][Alice] = Alice != SPONSORSHIP_ADDRESS

• So it calls the _twabController.sponsor function to change Alice's delegated address to SPONSORSHIP_ADDRESS.

• The attack now is over and Bob managed to change Alice delegatee to SPONSORSHIP_ADDRESS meaning that now she is not included in the prize distribution anymore (has 0 chances to win).

Alice could find out about this after a while and change the delegation but until that time she will be thinking that she's participating in the lottery when in fact she's not.
And Bob can repeat the same attack on other users who also deposited into the Vault.

Tools Used

Manual review

Recommended Mitigation Steps

There are many ways to solve this issue but the simplest one is to add a check in the _sponsor function which changes the delegation only if : _receiver == msg.sender.

The function _sponsor should be updated as follows :

function _sponsor(uint256 _assets, address _receiver) internal returns (uint256) {
    uint256 _shares = deposit(_assets, _receiver);
    // @audit only change delegation when caller is same as _receiver
    if (
      msg.sender == _receiver &&
      _twabController.delegateOf(address(this), _receiver) != _twabController.SPONSORSHIP_ADDRESS()
    ) {
      _twabController.sponsor(_receiver);
    }
    
    emit Sponsor(msg.sender, _receiver, _assets, _shares);
    
    return _shares;
}

Assessed type

Other

[c4-judge]

Picodes marked the issue as primary issue

[c4-sponsor]

asselstine marked the issue as sponsor confirmed

[Picodes]

Keeping High severity, considering this is an instance of "loss of yield"

[c4-judge]

Picodes marked the issue as satisfactory

[c4-judge]

Picodes marked issue #408 as primary and marked this issue as a duplicate of 408

[PierrickGT]

I've removed the receiver param in the following PR: GenerationSoftware/pt-v5-vault#19
This way, only the msg.sender can sponsor the Vault by depositing into it and delegating to the sponsorship address if it is not already the case.
If the user wants to deposit on behalf of another user, he can still use the deposit function. Funds will then be delegated to any address set by the receiver.