code-423n4 / 2024-07-reserve-validation

0 stars 0 forks source link

MinTradeVolume is subtracted more times than it should #135

Closed c4-bot-5 closed 1 month ago

c4-bot-5 commented 2 months ago

Lines of code

https://github.com/code-423n4/2024-07-reserve/blob/3f133997e186465f4904553b0f8e86ecb7bbacbf/contracts/p1/mixins/RecollateralizationLib.sol#L139-L200

Vulnerability details

Impact

wrong calculation of UoABottom which leads to wrong basketRange leading to wrong buy and sell amounts for rebalance

Proof of Concept

In RecollateralizationLibP1.basketRange() UoABottom is decreased minTradeVolume, in expectancy that auctions can return some dust amount of sellToken back, but in the current system design there is only one sell token per auction but

uoaBottom += (val < ctx.minTradeVolume) ? 0 : val - ctx.minTradeVolume;

the above line is executed inside a for loop which consists of all tokens in registery, if there are more than one tokens surplus its subtracted for all of them but the problem is there will only be one token that will be a returned as dust amount in one auction i.e sellToken, so it should only be subtracted once after all tokens are looped through. This can lead to lower than required range.bottom as it depends on UoABottom

Tools Used

Manual Review

Recommended Mitigation Steps

--  uoaBottom += (val < ctx.minTradeVolume) ? 0 : val - ctx.minTradeVolume;
++  uoaBottom += (val < ctx.minTradeVolume) ? 0 : val;
            }
        }
++    uoaBottom -= ctx.minTradeVolume;

        // ==== (2/3) Add-in ctx.*BasketsHeld safely ====

the following changes should be made in place of the current code

Assessed type

Error