Alpha - Miscalculation in Ticks in forwardAll() Results in Significant Loss to the protocol

[sherlock-admin2]

Alpha

Medium

Miscalculation in Ticks in forwardAll() Results in Significant Loss to the protocol

Summary

The forwardAll() function in the StreamEscrow.sol contract has a miscalculation issue where the protocol losses tokens. if the time elapsed since lastForwardTimestamp exceeds minimumTickDuration, the function only transfers the ethStreamedPerTick value for a single tick, even when multiple ticks have passed.

Root Cause

The primary issue lies within the forwardAll() function. When auctions take longer than 24 hours due to competitive bidding, and the forwardAll() function is called after a significant delay (i.e., block.timestamp > lastForwardTimestamp + minimumTickDuration), the contract sends ETH to the treasury for only one tick instead of multiple ticks.

Relevant code snippet:
https://github.com/sherlock-audit/2024-11-nounsdao/blob/main/nouns-monorepo/packages/nouns-contracts/contracts/StreamEscrow.sol#L136-#L149

Internal pre-conditions

1. block.timestamp > lastForwardTimestamp + minimumTickDuration.
2. No user has called forwardAll().

External pre-conditions

No response

Attack Path

1. An auction is created, and after successful bids, part of the bid amount is sent to the treasury, while the remainder is deposited into the StreamEscrow contract.
2. For every tick, a fixed amount (ethStreamedPerTick) is meant to be streamed from the escrow to the treasury.
3. If forwardAll() is not called for multiple ticks (e.g., 5 ticks), and it is later called:
   • The contract erroneously transfers ETH equivalent to one tick instead of the accumulated amount for all 5 ticks.
4. As a result:
   • Stream owners can withdraw more value when calling cancelStream().
   • The treasury suffers a loss of ETH intended for those ticks.

Impact

This issue causes:

1. Excess Tokens for Stream Owners: Stream owners receive more tokens than intended.
2. Treasury Losses: The treasury loses significant ETH, disrupting the allocation mechanism and potentially impacting the protocol's economic model.

PoC

No response

Mitigation

To resolve this issue:

1. Calculate the number of ticks that have passed between block.timestamp and lastForwardTimestamp.
2. Transfer the corresponding ETH amount for all elapsed ticks to the treasury.
3. Increment the tick count accordingly.

function forwardAll() public {
    // silently fail if at least a day hasn't passed. this is in order not to revert auction house.
    if (block.timestamp < lastForwardTimestamp + minimumTickDuration) {
        return;
    }

+  uint256 no_of_ticks = (block.timestamp - lastForwardTimestamp) / minimumTickDuration;
    lastForwardTimestamp = toUint48(block.timestamp);

 + sendETHToTreasury(ethStreamedPerTick * no_of_ticks); // changed

 +   uint32 newTick;
 +   uint128 ethPerTickEnded;
 +   for(uint i = 0; i < no_of_ticks; i++){
 +     (newTick, ethPerTickEnded) = increaseTicksAndFinishStreams();
 +   }

    emit StreamsForwarded(newTick, ethPerTickEnded, ethStreamedPerTick, lastForwardTimestamp);
    }