PoolTogether - minhtrng's results

Lines of code

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

Vulnerability details

Impact

Lack of access-control allows anyone to mint the yield fee to himself.

Proof of Concept

Vault.mintYieldFee does not restrict the _recipient yield fees:

  function mintYieldFee(uint256 _shares, address _recipient) external {
    _requireVaultCollateralized();
    if (_shares > _yieldFeeTotalSupply) revert YieldFeeGTAvailable(_shares, _yieldFeeTotalSupply);

    _yieldFeeTotalSupply -= _shares;
    _mint(_recipient, _shares);

    emit MintYieldFee(msg.sender, _recipient, _shares);
  }

Tools Used

Manual Review

Recommended Mitigation Steps

Dont use an extra parameter for the recipient and use the contracts _yieldFeeRecipient instead.

Assessed type

Access Control

Lines of code

https://github.com/GenerationSoftware/pt-v5-vault/blob/b1deb5d494c25f885c34c83f014c8a855c5e2749/src/Vault.sol#L988 https://github.com/GenerationSoftware/pt-v5-vault/blob/b1deb5d494c25f885c34c83f014c8a855c5e2749/src/Vault.sol#L480-L482

Vulnerability details

Impact

Anyone can delegate someone elses balance to the sponsorship address, increasing their own likelihood of winning, while voiding the victims chance.

Proof of Concept

The issue is in the call-chain starting with Vault.sponsor:

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

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);
}

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

function _delegate(address _vault, address _from, address _to) internal {
    address _currentDelegate = _delegateOf(_vault, _from);
    
    ...

    _transferDelegateBalance(
      _vault,
      _currentDelegate,
      _to,
      uint96(userObservations[_vault][_from].details.balance)
    );

Essentially anyone can call Vault.sponsor to deposit any amount of assets (even 0) to any receiver and the TwabController will then re-delegate the whole balance from the current delegate of the receiver to the SPONSORSHIP_ADDRESS.

Tools Used

Manual Review

Recommended Mitigation Steps

Sponsoring should be restricted to msg.sender as receiver

Assessed type

Invalid Validation

Lines of code

https://github.com/GenerationSoftware/pt-v5-twab-controller/blob/0145eeac23301ee5338c659422dd6d69234f5d50/src/TwabController.sol#L554-L570

Vulnerability details

Impact

An edge case in the TwabController._transferBalance can cause total balance for a vault account to decrease although it did not actually decrease. This will cause the sum of individual delegateBalances for a vault to be greater than the registered total for that vault. This again will skew the odds in favor of winning a price, causing the reserve to be drained faster over time than intended.

Proof of Concept

The TwabController._transferBalance function handles the case incorrectly where _to is equal to the SPONSORSHIP_ADDRESS. First, assume the _from address is an address that has a balance and delegates to itself (the default). Then the function will decrease the balance and delegateBalance of _from. It will also decrease the total balances of the vault account (note that _toDelegate will be SPONSORSHIP_ADDRESS due to default):

    if (_from != address(0)) {
      bool _isFromDelegate = _fromDelegate == _from;

      _decreaseBalances(_vault, _from, _amount, _isFromDelegate ? _amount : 0);

      if (!_isFromDelegate && _fromDelegate != SPONSORSHIP_ADDRESS) {
        _decreaseBalances(_vault, _fromDelegate, 0, _amount);
      }

      if (
        _to == address(0) ||
        (_toDelegate == SPONSORSHIP_ADDRESS && _fromDelegate != SPONSORSHIP_ADDRESS)
      ) {
        _decreaseTotalSupplyBalances(
          _vault,
          _to == address(0) ? _amount : 0,
          (_to == address(0) && _fromDelegate != SPONSORSHIP_ADDRESS) ||
            (_toDelegate == SPONSORSHIP_ADDRESS && _fromDelegate != SPONSORSHIP_ADDRESS)
            ? _amount
            : 0
        );
      }
    }

Then, the balance and delegateBalance of SPONSORSHIP_ADDRESS will be increased by the same amount. This is not supposed to happen in the first place, as this address is not meant to have any balances. However, the issue described under #impact is due to the fact that the total balances of the vault account will not be adjusted, because the condition _toDelegate != SPONSORSHIP_ADDRESS is not met:

    if (_to != address(0)) {
      bool _isToDelegate = _toDelegate == _to;

      _increaseBalances(_vault, _to, _amount, _isToDelegate ? _amount : 0);

      if (!_isToDelegate && _toDelegate != SPONSORSHIP_ADDRESS) {
        _increaseBalances(_vault, _toDelegate, 0, _amount);
      }

      if (
        _from == address(0) ||
        (_fromDelegate == SPONSORSHIP_ADDRESS && _toDelegate != SPONSORSHIP_ADDRESS)
      ) {
        _increaseTotalSupplyBalances(
          _vault,
          _from == address(0) ? _amount : 0,
          (_from == address(0) && _toDelegate != SPONSORSHIP_ADDRESS) ||
            (_fromDelegate == SPONSORSHIP_ADDRESS && _toDelegate != SPONSORSHIP_ADDRESS)
            ? _amount
            : 0
        );
      }
    }

The ratio of _userTwab and _vaultTwabTotalSupply plays a role when determining a winner in TierCalculationLib.isWinner and the underlying model assumes the invariant to hold true:

    uint256 constrainedRandomNumber = _userSpecificRandomNumber % (_vaultTwabTotalSupply);
    uint256 winningZone = calculateWinningZone(_userTwab, _vaultContributionFraction, _tierOdds);

If the sum of individual twabs is higher than the sum, there will be more winners than intended, causing the described drainage of the reserve.

Tools Used

Manual Review

Recommended Mitigation Steps

Disallow _to to be SPONSORSHIP_ADDRESS

Assessed type

Invalid Validation