code-423n4 / 2023-07-pooltogether-findings

12 stars 7 forks source link

The liquidate function lets the caller mint amountOut tokens without providing any #376

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

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

Vulnerability details

Impact

Ther are a lot of check to ensure the parameters of the liquidate function are correct. However, it does not checki for _amountIn to NOT be 0, thus it lets the caller proceed and mint _amountOut tokens to _account without providing any

Proof of Concept

function liquidate(
    address _account,
    address _tokenIn,
    uint256 _amountIn,
    address _tokenOut,
    uint256 _amountOut
  ) public virtual override returns (bool) {
    _requireVaultCollateralized();
    if (msg.sender != address(_liquidationPair)) //  modify en storage y blolquear
      revert LiquidationCallerNotLP(msg.sender, address(_liquidationPair));
    if (_tokenIn != address(_prizePool.prizeToken()))
      revert LiquidationTokenInNotPrizeToken(_tokenIn, address(_prizePool.prizeToken()));
    if (_tokenOut != address(this)) //  no sería $POOL
      revert LiquidationTokenOutNotVaultShare(_tokenOut, address(this));
    if (_amountOut == 0) revert LiquidationAmountOutZero();

    // what about _amountIn ??
    ...

The only place it is used is in

_prizePool.contributePrizeTokens(address(this), _amountIn); // #L570 in PrizePool.sol

which calls

function contributePrizeTokens(address _prizeVault, uint256 _amount) external returns (uint256) {
    uint256 _deltaBalance = prizeToken.balanceOf(address(this)) - _accountedBalance();
    if (_deltaBalance < _amount) {
      revert ContributionGTDeltaBalance(_amount, _deltaBalance);
    }
    DrawAccumulatorLib.add(
      vaultAccumulator[_prizeVault],
      _amount,
      lastClosedDrawId + 1,
      smoothing.intoSD59x18()
    );
    DrawAccumulatorLib.add(
      totalAccumulator,
      _amount,
      lastClosedDrawId + 1,
      smoothing.intoSD59x18()
    );
    emit ContributePrizeTokens(_prizeVault, lastClosedDrawId + 1, _amount);
    return _deltaBalance;
  }

and neither validate the _amount parameter, thus it proceeds to the mint call in line 584 of Vault.sol without reverting

Tools Used

Manual analysis

Recommended Mitigation Steps

Just check _amountIn to not be 0

Assessed type

Invalid Validation

Picodes commented 1 year ago

This largely depends on the liquidator's implementation, which is out of scope

c4-judge commented 1 year ago

Picodes marked the issue as primary issue

c4-judge commented 1 year ago

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

c4-judge commented 1 year ago

Picodes marked the issue as unsatisfactory: Out of scope