code-423n4 / 2024-04-renzo-findings

12 stars 8 forks source link

Incorrect calculateTVLs formula #197

Closed howlbot-integration[bot] closed 6 months ago

howlbot-integration[bot] commented 6 months ago

Lines of code

https://github.com/code-423n4/2024-04-renzo/blob/519e518f2d8dec9acf6482b84a181e403070d22d/contracts/RestakeManager.sol#L318

Vulnerability details

Impact

calculateTVLs function contains an error than can lead to incorrect calculations of TVL, leading to overvaluation or undervaluation of user position.

Here are the lines of code in question:

https://github.com/code-423n4/2024-04-renzo/blob/519e518f2d8dec9acf6482b84a181e403070d22d/contracts/RestakeManager.sol#L318

instead of

if (!withdrawQueueTokenBalanceRecorded) {
                    totalWithdrawalQueueValue += renzoOracle.lookupTokenValue(
                        collateralTokens[i], 
                        collateralTokens[j].balanceOf(withdrawQueue)
                    );
                }

the formula should be

if (!withdrawQueueTokenBalanceRecorded) {
                    totalWithdrawalQueueValue += renzoOracle.lookupTokenValue(
                        collateralTokens[j], 
                        collateralTokens[j].balanceOf(withdrawQueue)
                    );
                }

i is the index of 'operatorDelegators' list, while j is the index of 'collateralTokens' list

i is used in outer loop and j in inner loop, with 'withdrawQueueTokenBalanceRecorded' set to true after first outer loop, skipping this block in subsequent outer loops.

This lead to all totalWithdrawalQueueValue TVL being calculated with renzoOracle.lookupTokenValue argument set to collateralTokens[0], using first token of collateralTokens list for lookup, but values/amount of all other tokens, leading to incorrect totalWithdrawalQueueValue and subsequently TVL calculations.

Proof of Concept

Assume there are two tokens accepted in Renzo, e.g. stETH, wBETH, with stETH being 'collateralTokens[0]' and wBETH 'collateralTokens[1]' Assume 100 units of value is in both, 50% in stETH and wBETH.

If stETH depegs by 20%, the true TVL in queue would be 0.50.8+0.5 = 0.9. Per current code, it would be 0.50.8+0.5*0.8 = 0.8, leading to unfair decrease in user share prices.

If instead wBETH depegs by 20%, the TVL would stay at 100, which would lead to unfair increase in user share prices and loss to protocol.

Tools Used

Manual review

Recommended Mitigation Steps

Correct the lookup index as described above.

Assessed type

Math

c4-judge commented 5 months ago

alcueca marked the issue as satisfactory

c4-judge commented 5 months ago

alcueca changed the severity to 2 (Med Risk)

c4-judge commented 5 months ago

alcueca changed the severity to 3 (High Risk)

c4-judge commented 5 months ago

alcueca changed the severity to 2 (Med Risk)

c4-judge commented 5 months ago

alcueca changed the severity to 3 (High Risk)