PoolTogether V5: Part Deux - Angry_Mustache_Man's results

Lines of code

https://github.com/GenerationSoftware/pt-v5-vault-boost/blob/9d640051ab61a0fdbcc9500814b7f8242db9aec2/src/VaultBooster.sol#L249-#L257

Vulnerability details

Impact

Accounting error in accruing at VaultBooster.sol will cause unexpected problems in VaultBooster.sol contract.

Proof of Concept

The _accrue function of VaultBooster.sol at :

https://github.com/GenerationSoftware/pt-v5-vault-boost/blob/9d640051ab61a0fdbcc9500814b7f8242db9aec2/src/VaultBooster.sol#L249-#L257

does not check whether the return variable of _computeAvailable(_tokenOut) i.e., available is greater than _tokenOut.balanceOf(address(this)).

One main problem that could occur is underflow error while withdrawing the boosted tokens

Let’s consider a case of emergency, where Owner is trying to withdraw all the boosted tokens from the contract at a time. He would call getBoost function to get information regarding available boosted tokens. Then he will try to withdraw all boosted tokens at a time . But it will cause problems as

(available balance of boosted tokens) > (_tokenOut.balanceOf(address(this)) 

And this line shown below which is present in withdraw function will cause an underflow error:

uint256 remainingBalance = availableBalance - _amount; 

Because the amount entered by Owner would be _boosts[_tokenOut].available which as we can see from above is less than (_tokenOut.balanceOf(address(this)).

Tools Used

Manual Review

Recommended Mitigation Steps

Add these modifications to _accrue() :

  function _accrue(IERC20 _tokenOut) internal returns (uint256) { 

    uint256 available = _computeAvailable(_tokenOut);   

  +   uint256 total =  (available > (_tokenOut.balanceOf(address(this))) ? (_tokenOut.balanceOf(address(this)) : available; 

    _boosts[_tokenOut].available = total.toUint144(); 

    _boosts[_tokenOut].lastAccruedAt = uint48(block.timestamp); 

  

    emit BoostAccrued(_tokenOut, available); 

 +   return total; 

  } 


## Assessed type

Error

Lines of code

https://github.com/GenerationSoftware/pt-v5-cgda-liquidator/blob/7f95bcacd4a566c2becb98d55c1886cadbaa8897/src/libraries/ContinuousGDA.sol#L39

Vulnerability details

Impact

Breaks the core functionality of the Liquidation Pair contract.

Usage of wrong formula for calculation of Continuous Gradual Dutch Auction results in wrong calculation of purchase price which is basically used to find the swapAmountIn during liquidations .

Proof of Concept

Statements from PoolTogether Code4rena docs:

The LiquidationPair prices yield liquidations using a periodic Continuous Gradual Dutch Auction. It's periodic in the sense that the auction runs in periods that will be aligned with the prize pool periods. At the beginning of each period, the CGDA adjusts the emissions rate and target price so that it adapts to changing market conditions.

PoolTogether implementation of Continuous Gradual Dutch Auction uses formula:

(k/(r))*(e**((lambda)*q)/r)-1)/(e**((lambda)*T)) 

at: https://github.com/GenerationSoftware/pt-v5-cgda-liquidator/blob/7f95bcacd4a566c2becb98d55c1886cadbaa8897/src/libraries/ContinuousGDA.sol#L39

The original formula of Continuous Gradual Dutch Auction:

(k/(lambda))*(e**((lambda)*q)/r)-1)/(e**((lambda)*T)) 

First Term is k/(lambda) not (k/(r).

Reference :

Official Paradigm Article on CGDA’s which shows the correct formula

Official Github Link of Correct Implementaion of CGDA’s by Paradigm

Tools Used

Manual Review

Recommended Mitigation Steps

Reimplement the first term of Formula as k/(lambda) that is k.div(_decayConstant).

Assessed type

Error