PoolTogether V5: Part Deux - 14si2o_Flint's results

Lines of code

https://github.com/GenerationSoftware/pt-v5-vault-boost/blob/9d640051ab61a0fdbcc9500814b7f8242db9aec2/src/VaultBooster.sol#L167-L176 https://github.com/GenerationSoftware/pt-v5-vault-boost/blob/9d640051ab61a0fdbcc9500814b7f8242db9aec2/src/VaultBooster.sol#L246-L257

Vulnerability details

Impact

The VaultBooster.sol contract allows someone to liquidate tokens for a vault and improve the vault's chance of winning. The prizePool.prizeToken is set in the constructor and liquidationPair set in the setBoost function.

However, the external deposit function has no access control or input validation, which means users of the protocol can deposit any IERC20 token in the Vaultbooster which then gets stuck if the token is not the liquidationPair.

The deposited token will get added to _boosts but is not used in the calculations of liquidationPair and the withdraw function can only be used by the Owner with a hardcoded withdrawal address of msg.sender.

Consequently, there is no viable way for users to recover funds deposit deposited in the Vaultbooster when they have deposited the wrong IERC20 token.

Proof of Concept

https://github.com/GenerationSoftware/pt-v5-vault-boost/blob/9d640051ab61a0fdbcc9500814b7f8242db9aec2/src/VaultBooster.sol#L167-L176

https://github.com/GenerationSoftware/pt-v5-vault-boost/blob/9d640051ab61a0fdbcc9500814b7f8242db9aec2/src/VaultBooster.sol#L246-L257

When calling the deposit function with a wrong IERC20 token (for example WETH), the internals functions _accrue and _computeAvailable will be called to obtain and set Boost parameters. Even though there is no Boost in the _boosts mapping for that IERC20 token, both functions will not revert and simply add a new Boost to the mapping.

Below are two small Remix tests which show that neither _accrue nor _computeAvailable will revert.

contract TestAccrue {

    struct Boost {
        address liquidationPair;
        uint144 available;
        uint48 lastAccruedAt;
    }

    address weth = 0xC02aaA39b223FE8D0A0e5C4F27eAD9083C756Cc2;
    mapping(IERC20 => Boost) internal _boosts;

    function deposit(IERC20 _token, uint144 amount) public {
        _boosts[_token].available = amount;
    }

    function getBoosts() public view returns(uint256) {
        return _boosts[IERC20(weth)].available;
    }
}

contract TestComputeAvailable {

    struct Boost {
        address liquidationPair;
        uint144 available;
        uint48 lastAccruedAt;
    }

    address weth = 0xC02aaA39b223FE8D0A0e5C4F27eAD9083C756Cc2;
    mapping(IERC20 => Boost) internal _boosts;

    function compute(IERC20 _token) public view returns(uint256) {
        Boost memory boost = _boosts[_token];
        return boost.available;

    }
}

Tools Used

Manual review, Remix

Recommended Mitigation Steps

Add a modifier to deposit so that it will revert if the IERC20 token being deposited does not equal the liquidationPair set by the setBoost function.

Assessed type

Access Control