Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Lack of sequencer uptime checks can lead to dutch auctions executing at bad prices or failing #18

Open
c4-bot-2 opened this issue Aug 3, 2024 · 0 comments
Labels
2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working 🤖_33_group AI based duplicate group recommendation sufficient quality report This report is of sufficient quality

Comments

@c4-bot-2
Copy link
Contributor

c4-bot-2 commented Aug 3, 2024

Lines of code

https://github.com/code-423n4/2024-07-reserve/blob/825e5a98a8c94a7b4458b3b359a507edb9e662ba/contracts/plugins/trading/DutchTrade.sol#L199-L205

Vulnerability details

Proof of Concept

First, per the readMe, protocol is to be deployed on multiple chains, optimistic L2s included, see https://github.com/code-423n4/2024-07-reserve/blob/825e5a98a8c94a7b4458b3b359a507edb9e662ba/README.md#L140

| Chains the protocol will be deployed on | Ethereum,Arbitrum,Base,Optimism |

The protocol implements Dutch auction trades via DutchTrade.sol. The auction mechanism is designed with a specific price decay structure over time: see the docs for a short map on how these auctions should work.

// A dutch auction in 4 parts:
//   1.  0% -  20%: Geometric decay from 1000x the bestPrice to ~1.5x the bestPrice
//   2. 20% -  45%: Linear decay from ~1.5x the bestPrice to the bestPrice
//   3. 45% -  95%: Linear decay from the bestPrice to the worstPrice
//   4. 95% - 100%: Constant at the worstPrice

However, the contract does not include any checks for sequencer uptime. This can be problematic because network outages and reorgs happen quite often with optimistic L2s.

Now to place bids we have https://github.com/code-423n4/2024-07-reserve/blob/825e5a98a8c94a7b4458b3b359a507edb9e662ba/contracts/plugins/trading/DutchTrade.sol#L199-L205

    function bid() external returns (uint256 amountIn) {
        require(bidder == address(0), "bid already received");
        assert(status == TradeStatus.OPEN);

        // {buyTok/sellTok}
        uint192 price = _price(uint48(block.timestamp)); // enforces auction ongoing //@audit
..snip
    }

Take a look at the _price() function, which calculates the auction price based on the current timestamp:

function _price(uint48 timestamp) private view returns (uint192) {
  uint48 _startTime = startTime; // {s} gas savings
  uint48 _endTime = endTime; // {s} gas savings
  require(timestamp >= _startTime, 'auction not started');
  require(timestamp <= _endTime, 'auction over');

  uint192 progression = divuu(timestamp - _startTime, _endTime - _startTime); // @audit

  // ...snip
}

Now, if there's a sequencer downtime, which we should agree the likelihood to be substantial considering the various chains protocol is to deploy to the timestamp used for price calculation could be significantly off, leading to incorrect pricing.

Impact

Auctions could execute at unfavorable prices if there's a significant delay between when a bid is submitted and when it's processed due to sequencer downtime, even worse these auctions could fail entirely if they end during a period of sequencer downtime, as no bids would be able to be placed.

Since "sequencer downtime" can be seen as a twin window to reorg issues, would be key to note that since create2 is not being used for these deployments of the auctions then we have issues where users might bid on the wrong auction due to a swap of the addresses since the reorg could switch addresses for 2 different auctions. Another subtle scenario is say a user would like to mint right after the RToken was deployed, a reorg might cause for the deployment of a different RToken and trap the users' funds in it.

Tool used

Recommended Mitigation Steps

Consider implementing a check for sequencer uptime using Chainlink's L2 Sequencer Uptime Feed. This can be used to ensure that the sequencer was online during the entire duration of an auction and then add a mechanism to extend or restart auctions if there was significant sequencer downtime during the auction period, and for the switch in address case, using create2 should sort this out.

Assessed type

Context

@c4-bot-2 c4-bot-2 added 2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working labels Aug 3, 2024
c4-bot-10 added a commit that referenced this issue Aug 3, 2024
@c4-bot-12 c4-bot-12 added the 🤖_33_group AI based duplicate group recommendation label Aug 19, 2024
howlbot-integration bot added a commit that referenced this issue Aug 20, 2024
@howlbot-integration howlbot-integration bot added the sufficient quality report This report is of sufficient quality label Aug 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working 🤖_33_group AI based duplicate group recommendation sufficient quality report This report is of sufficient quality
Projects
None yet
Development

No branches or pull requests

2 participants