RubiconMarket._buys will not work for V1 offers due to the reversion in cancel method.

[code423n4]

Lines of code

https://github.com/code-423n4/2023-04-rubicon/blob/511636d889742296a54392875a35e4c0c4727bb7/contracts/RubiconMarket.sol#L1201-L1202

Vulnerability details

Impact

RubiconMarket._buys tries to cancel dust remaining offers, but it fails for V1 offers. So it blocks RubiconMarket._buys.

Proof of Concept

In RubiconMarket._buys, it tries to cancel an offer with dust pay token amount after fulfilling. So it sets dustId so it can pass the RubiconMarket.can_cancel modifier.

        if (
            isActive(id) &&
            offers[id].pay_amt < _dust[address(offers[id].pay_gem)]
        ) {
            dustId = id; //enable current msg.sender to call cancel(id)
            cancel(id);
        }

But when we move into cancel method, there is another validation. RubiconMarket.cancel will call SimpleMarket.cancel, and SimpleMarket.cancel tries to return dust pay token amount to owner.

        _offer.owner == address(0) && msg.sender == _offer.recipient
            ? require(_offer.pay_gem.transfer(_offer.recipient, _offer.pay_amt))
            : require(_offer.pay_gem.transfer(_offer.owner, _offer.pay_amt));

In this case, msg.sender is different from _offer.recipient so SimpleMarket.cancel will treat this as a V2 offer, while it can be a valid V1 offer. So for a V1 offer, it tries to send pay token amount to _offer.owner, and it is address(0) for V1 offers. So it will revert and SimpleMarket.cancel will not work although it passes RubiconMarket.can_cancel modifier. This will block _buy method.

Tools Used

Manual Review

Recommended Mitigation Steps

In SimpleMarket.cancel, we should refund pay token amount to _offer.recipient when _offer.owner is address(0).

[c4-pre-sort]

0xSorryNotSorry marked the issue as duplicate of #1281

[c4-judge]

HickupHH3 changed the severity to 3 (High Risk)

[c4-judge]

HickupHH3 marked the issue as satisfactory

[c4-judge]

HickupHH3 marked the issue as selected for report