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

12 stars 7 forks source link

DOS Vault.liquidate() if _yieldFeePercentage = FEE_PRECISION. #353

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#L572-L576 https://github.com/GenerationSoftware/pt-v5-vault/blob/b1deb5d494c25f885c34c83f014c8a855c5e2749/src/Vault.sol#L1218-L1223

Vulnerability details

Impact

Vault.sol is set to Yield fee percentage represented in integer format with 9 decimal places (i.e. 10000000 = 0.01 = 1%). You can install it either in the constructor (https://github.com/GenerationSoftware/pt-v5-vault/blob/b1deb5d494c25f885c34c83f014c8a855c5e2749/src/Vault.sol#L277) or later (https://github.com/GenerationSoftware/ pt-v5-vault/blob/b1deb5d494c25f885c34c83f014c8a855c5e2749/src/Vault.sol#L1218-L1223). In this case, during installation, the following condition is checked: if (yieldFeePercentage_ > FEEPRECISION) { revert YieldFeePercentageGTPrecision(yieldFeePercentage, FEE_PRECISION); }

It follows from the above condition that yieldFeePercentage_ can be equal to FEE_PRECISION.

In the liquidate function (https://github.com/GenerationSoftware/pt-v5-vault/blob/b1deb5d494c25f885c34c83f014c8a855c5e2749/src/Vault.sol#L572-L576) the yield fee balance accrued by _yieldfeercipient is incremented:

if (_yieldFeePercentage != 0) { _increaseYieldFeeBalance( (_amountOut * FEE_PRECISION) / (FEE_PRECISION - _yieldFeePercentage) - _amountOut ); }

In the calculation (_amountOut * FEE_PRECISION) / (FEE_PRECISION - _yieldFeePercentage) - _amountOut, if FEE_PRECISION == _yieldFeePercentage the value will be zero. This is a hello to the DOS liquidate function with FEE_PRECISION = _yieldFeePercentage.

Tools Used

Manual review

Recommended Mitigation Steps

When setting _yieldFeePercentage in the _setYieldFeePercentage() function (https://github.com/GenerationSoftware/pt-v5-vault/blob/b1deb5d494c25f885c34c83f014c8a855c5e2749/src/Vault.sol#L1218-L1223) add "=":

if (yieldFeePercentage_ >= FEEPRECISION) { revert YieldFeePercentageGTPrecision(yieldFeePercentage, FEE_PRECISION); }

Assessed type

DoS

c4-judge commented 1 year ago

Picodes changed the severity to QA (Quality Assurance)

c4-judge commented 1 year ago

This previously downgraded issue has been upgraded by Picodes

c4-judge commented 1 year ago

This previously downgraded issue has been upgraded by Picodes

c4-judge commented 1 year ago

Picodes marked the issue as primary issue

Picodes commented 1 year ago

Should be QA due to the low probability and the fact that this is an admin configuration

c4-sponsor commented 1 year ago

asselstine marked the issue as sponsor confirmed

c4-judge commented 1 year ago

Picodes changed the severity to QA (Quality Assurance)