Submitted by carrotsmuggler, also found by joestakey, Udsen, brgltd, alexzoid, zhuXKET, 0xBeirao, nirlin, halden, McToady, LaScaloneta, immeas, Evo, favelanky, 0Kage, Nyx, Nyx, Udsen, Neon2835, __141345__, bin2chen, whoismatthewmc1, ast3ros, whoismatthewmc1, 0xfusion, CryptoCraze, CryptoCraze, Juntao, SaeedAlipoor01988, Lilyjjo, 0xPiercer, MalfurionWhitehat, CRYP70, 3agle, SaeedAlipoor01988, ktg, ladboy233, ladboy233, sces60107, sces60107, popular00, anodaram, mrpathfindr, and mrpathfindr.
https://github.com/code-423n4/2023-04-rubicon/blob/511636d889742296a54392875a35e4c0c4727bb7/contracts/RubiconMarket.sol#L1079 https://github.com/code-423n4/2023-04-rubicon/blob/511636d889742296a54392875a35e4c0c4727bb7/contracts/RubiconMarket.sol#L1039
The Rubicon market has two functions that are interesting: buyAllAmount and sellAllAmount. These functions are responsible for using up offers until the entire amount is bought or sold. The contract stores offers in a sorted linked list, each with its unique id, and sorted according to the price. This ensures that the head of the linked list has the best price, or the best offer for either sell or buy. This head is referenced in the code by the mapping _best[address][address] and further worse, offers can be navigated to by calling best.next, similar to a linked list navigation.
When buyAllAmount is called, a while loop is ran, and the best offer is taken out in each iteration using the function getBestOffer() similar to the snippet below:
The offers are then filled one after the other. However, if an offer is unfillable, or reverts, then the whole transaction reverts. This breaks the functionality of the buyAllAmount function and creates a DOS attack vector.
The standard openzeppelin implementation of the ERC20 token does not support transfers to the zero address. If a transaction tries to send tokens to 0 address, it reverts with ERC20: transfer to the zero address. Thus, if an offer is created where the receiver of the payment is zero address, that offer can be unfillable. If this offer is created such that it is the best possible price, then this offer will exist at the head of the linked list in the _best variable and thus will be the first offer looked into. Since this offer is unfillable, the entire transaction will revert.
When creating an offer, the user can specify two addresses, the owner address and the recipient address. The contract does not sanitize these addresses and even accepts 0 address in these fields. These are also the addresses where the payment token is sent when the order is filled.
Thus, a malicious user can create a buy/sell order of a small volume but at a very good price. They can set both the owner and the recipient to 0 when calling offer(). This will create a lucrative unfillable order which will always be called on first. This will lead to all transactions reverting due to this single unfillable offer.
The impact is even more pronounced for the Position.sol contract. Unlike normal users, this contract does not have the ability to select offers by id, and is only filled using buyAllAmount or sellAllAmount. A DOS attack, as described above, will break the functionality of the two mentioned functions and break the whole Position contract, as a result. This can lead to large losses where users are unable to unwind positions before liquidations.
This also has a secondary impact of blocking the creation of future orders. As can be seen here, in the _matcho function, when new offers are created they are first matched with existing offers in the same range.  If there is still a balance remaining, a new offer is created. Now if the existing offers are such unfillable orders, it would also block the creation of further new orders, causing another functionality DOS.
The POC shows that if the recipient and owner addresses are both set to 0, the order cannot be filed. This order will exist in the linked list sorted by price and thus will be the first order to be filled.
First, change the code in deployRubiconProtocolFixture() where the orders are created here to the following order:
This creates an unfillable order. Now try running the provided test in can take an offer with 0 makerFee by using the command: hardhat test --grep "0 makerFee".
This test fails with ERC20: transfer to the zero address error, since the order is unfillable. The test body is also copied here as reference:
This shows unfillable orders can be created. If this order exists at the top, it will revert calls from the Position.sol contract.
Hardhat
This requires multiple mitigation steps:
daoio (Rubicon) confirmed
HickupHH3 (judge) commented:
HickupHH3 (judge) decreased severity to Medium
carrotsmuggler (warden) commented:
HickupHH3 (judge) increased severity to High and commented:
