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

12 stars 7 forks source link

`Vault.liquidate()` will revert when `_yieldFeePercentage` is 100% #385

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/GenerationSoftware/pt-v5-vault/tree/b1deb5d494c25f885c34c83f014c8a855c5e2749/src/Vault.sol#L574 https://github.com/GenerationSoftware/pt-v5-vault/tree/b1deb5d494c25f885c34c83f014c8a855c5e2749/src/Vault.sol#L1218-L1224

Vulnerability details

Impact

When the yield fee percentage is 100%, the liquidate() method will not work.

Proof of Concept

In Vault.liquidate(), we get fee portion from share amount out and increase yield fee balance with it.

    (_amountOut * FEE_PRECISION) / (FEE_PRECISION - _yieldFeePercentage) - _amountOut

If _yieldFeePercentage is FEE_PRECISION, i.e. 100%, liquidate() will revert. And the edge case passes the validation in the set method.

  function _setYieldFeePercentage(uint256 yieldFeePercentage_) internal {
    if (yieldFeePercentage_ > FEE_PRECISION) {
      revert YieldFeePercentageGTPrecision(yieldFeePercentage_, FEE_PRECISION);
    }
    _yieldFeePercentage = yieldFeePercentage_;
  }

As a result, liquidate() will not work in the edge case when the fee percentage is 100%.

Tools Used

Manual Review

Recommended Mitigation Steps

We can input the original fee amount as a parameter instead of getting from _amountOut

Assessed type

Error

c4-judge commented 1 year ago

Picodes marked the issue as unsatisfactory: Overinflated severity

Picodes commented 1 year ago

The described impact doesn't qualify for Med severity