search

sherlock-audit / 2024-02-rubicon-finance-judging

5 stars 3 forks source link

irresponsible - Partition rounds up which can cause orders to be unfillable because of revert #31

Closed sherlock-admin2 closed 7 months ago

sherlock-admin2 commented 7 months ago

irresponsible

high

Partition rounds up which can cause orders to be unfillable because of revert

Summary

Partition rounds up which can cause orders to be unfillable because of revert

Vulnerability Detail

Partition function returns parts of input and output amounts to execute. This is how partial fill works in rubicon. The problem arises when we try to calculate outPart ,and both quantity and input.amount are equal. 

        uint256 outPart = quantity.mulDivUp(output[0].amount, input.amount);

Above we see how outPart is calculated, the problem is that we use mulDivUp this rounds up the result. This is a big problem because now outPart is now larger than output[0].amount. The reason why this is a problem is because when we try to call _validatePartition, we will revert.

        if (_quantity > _initIn || _outPart > _initOut)
            revert PartialFillOverflow();

Above is a snippet from the function _validatePartition, in which we see the revert. In the function as i have explained above, _outPart will be larger than _initOut and will cause the revert.

Impact

Why is this a problem?

  1. certain order configurations will never be able to be filled, when they otherwise should.
  2. certain decayed input will not be able to be filled, forcing fillers to wait for input to decay further. This is a loss of funds for the user, if the filler wants to fill but cannot because of this bug, he must then wait for more decay in order to fill the order. In some scenarios, this bug is forcing fillers to fill at lower rates because of this bug.

    Code Snippet

    https://github.com/sherlock-audit/2024-02-rubicon-finance/blob/11cac67919e8a1303b3a3177291b88c0c70bf03b/gladius-contracts-internal/src/lib/PartialFillLib.sol#L85

    Tool used

Manual Review

Recommendation

use mulDivDown instead

Duplicate of #29

sherlock-admin commented 7 months ago

1 comment(s) were left on this issue during the judging contest.

0xAadi commented: