PoolTogether - oxchsyston's results

Lines of code

https://github.com/GenerationSoftware/pt-v5-vault/blob/b1deb5d494c25f885c34c83f014c8a855c5e2749/src/VaultFactory.sol#L55 https://github.com/GenerationSoftware/pt-v5-vault/blob/b1deb5d494c25f885c34c83f014c8a855c5e2749/src/Vault.sol#L254

Vulnerability details

Impact

Malicious vaultowner can depoy vault using malicious twabcontroller, yieldvault and prizepool contracts that can put users' funds at risk.

Proof of Concept

In the contract vaultfactory.sol, the function deployvault is used to deploy new vaults and then push it to the allvaults array. The addresses of the twabcontroller, yieldvault and prizepool were passed as input in the function but were not validated before the vault was deployed, in the cunstructor() of the vaults.sol contract, the if statement only checks that the addresses are not the zero address, but does not ensure that the addresses are the right ones.

 function deployVault(
    IERC20 _asset,
    string memory _name,
    string memory _symbol,
    TwabController _twabController,
    IERC4626 _yieldVault,
    PrizePool _prizePool,
    address _claimer,
    address _yieldFeeRecipient,
    uint256 _yieldFeePercentage,
    address _owner
  ) external returns (address) {
    Vault _vault = new Vault(
      _asset,
      _name,
      _symbol,
      _twabController,
      _yieldVault,
      _prizePool,
      _claimer,
      _yieldFeeRecipient,
      _yieldFeePercentage,
      _owner
    );

    allVaults.push(_vault);
    deployedVaults[_vault] = true;

    emit NewFactoryVault(_vault, VaultFactory(address(this)));

    return address(_vault);
  }

Tools Used

Manual review

Recommended Mitigation Steps

ensure proper validation since these parameters are passed in manually which can possibly lead to some typos.

Assessed type

Invalid Validation