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

0 stars 0 forks source link

Leak of value due to reversed rounding in req struct newBatchAuction function in Broker.sol contract #185

Closed c4-bot-1 closed 1 month ago

c4-bot-1 commented 1 month ago

Lines of code

https://github.com/code-423n4/2024-07-reserve/blob/3f133997e186465f4904553b0f8e86ecb7bbacbf/contracts/p1/Broker.sol#L254-L255

Vulnerability details

Impact

reversed rounding in Broker.sol contract in newBatchAuction function leading to leakage of value

Proof of Concept

In the req.sellAmount we round to CEIL, And in req.minBuyAmount we round to FLOOR

     function newBatchAuction(TradeRequest memory req, address caller) private returns (ITrade) {

         if (maxQty > GNOSIS_MAX_TOKENS) {
>>>          req.sellAmount = mulDiv256(req.sellAmount, GNOSIS_MAX_TOKENS, maxQty, CEIL);
>>>          req.minBuyAmount = mulDiv256(req.minBuyAmount, GNOSIS_MAX_TOKENS, maxQty, FLOOR);
         }

Tools Used

manual review

Recommended Mitigation Steps

as done in TradeLib.sol we should round sellAmount to FLOOR and minBuyAmount to CEIL

         // {*tok} => {q*Tok}
         req.sellAmount = s.shiftl_toUint(int8(trade.sell.erc20Decimals()), FLOOR);
         req.minBuyAmount = b.shiftl_toUint(int8(trade.buy.erc20Decimals()), CEIL);
         req.sell = trade.sell;
         req.buy = trade.buy;

Assessed type

Math