Skip to content

Latest commit

 

History

History
64 lines (34 loc) · 3.88 KB

049.md

File metadata and controls

64 lines (34 loc) · 3.88 KB

Bright Spruce Otter

Medium

forwardAllAndCreateStream() can be frontrunned to steal one tick worth of extra refunds

Summary

At every auction settlement, the AuctionHouse contract calls forwardAllAndCreateStream(). In forwardAll() logic, ethStreamedPerTick is first transferred to ethRecipient and then the currentTick is incremented.

This call is always made at a difference of 24 hours (== auction settlement duration) from the auctionHouse and forwardAll() only handles the fund transfer and increment logic if 24 hours have passed.

The problem is that if a user frontruns the forwardAllAndCreateStream() with a call to cancelStream() at the exact same time (ie. 24 hours since the lastForwardTimestamp, the user gets one tick worth of extra refund before the currentTick could be incremented. This causes a loss of 1 tick worth of stream amount for the DAO.

https://github.com/sherlock-audit/2024-11-nounsdao/blob/main/nouns-monorepo/packages/nouns-contracts/contracts/StreamEscrow.sol#L167-L186

Root Cause

cancelStream() does not check and increment the currentTick before calculating the deserved refund amount for a cancelled stream.

Internal pre-conditions

No response

External pre-conditions

No response

Attack Path

Lets assume that only one stream exists which has started just now, and its the start of the streamescrow so currentTick == 0.

  1. Noun owner has a stream set up with 15 ETH of total stream, ie. ethPerTick of 0.01 ETH
  2. Now 24 hours have passed since the stream was created
  3. An auction settlement call is made in the AuctionHouse contract, which calls forwardAllAndCreateStream() => forwardAll()
  4. The noun owner sees this txn and frontruns it with a call to cancelStream()
  5. Because cancelStream() does not check/ increment the currentTick, the refund calculation assumes that number of ticks left == 1500 - 0 == 1500
  6. The noun owner gets a refund of 15 ETH ie. the full amount even though 24 hours had passed since the stream was created. This is because the logic decreases ethStreamedPerTick without first checking that the current tick has ended (and if so it needs to send this last tick funds also to the DAO)
  7. This results in the DAO getting nothing from the streaming even though the noun bid was escrowed for a full tick duration

This can happen for any streams in the whole lifecycle of the streamEscrow contract, and the DAO will lose 1 tick worth of funds that they deserve as per the mechanics of the system.

For our current example, the loss will be 0.01 ETH (15/1500) and in percentage terms all streams can lose (1/1500 * 100) ie. 0.0666~ % of a single auction stream amount.

Impact

Dao loses one tick worth of funds because one extra tick worth of funds are refunded as the currentTick is not correctly incremented before the refund calculation.

The extent of loss is 0.066~% so this is a medium severity issue.

PoC

No response

Mitigation

In cancelStream(), plug a call to forwardAll() to make sure that if the time has passed since lastForwardTimestamp then the tick should be incremented and funds sent to the treasury before doing any refund calculations (so that the correct ethStreamedPerTick is utilized first)

Because forwardAll() is public, this issue will be prevented if someone calls forwardAll() before the cancelStream() call could execute. But no one has an incentive to do that. There is no bot as the protocol has specifically stated that they are solely depending upon the auction settlement transaction itself to forward the stream. See https://mirror.xyz/verbsteam.eth/GYWRLqAC0heMC_2Fhc0ez0Mchxp_MCUIjoyt8UPGwrs#:~:text=Why%20use%20Auction%20House%20as%20the%20clock%3F

And if some user or a user-setup bot tries to waste gas and call forwardAll(), the attacker can also frontrun this transaction easily. The only solution is to plug forwardAll() in cancelStream().