Pablo - DAO won't receive full ETH streams in edge case

[sherlock-admin3]

Pablo

Medium

DAO won't receive full ETH streams in edge case

Summary

In the StreamEscrow.cancelStream() function, owner of noun can cancel the stream and receive unstreamed funds. When at least minimumTickDuration seconds has passed since last forward, the function StreamEscrow.forwardAll() is invoked and all pending ETH streams are sending to DAO.

However, if owner of noun want to cancel his stream at the same time, he can front-run this transaction and don't send ethPerTick amount of ETH to DAO. As result, DAO won't get stream it deserves.

Root Cause

In the StreamEscrow.sol:167 function, owner of noun can cancel the stream and receive unstreamed funds.

167 function cancelStream(uint256 nounId) public {
        require(isStreamActive(nounId), 'stream not active');

        // transfer noun to treasury
        nounsToken.transferFrom(msg.sender, nounsRecipient, nounId);

        // cancel stream
        streams[nounId].canceled = true;
        Stream memory stream = streams[nounId];
        ethStreamedPerTick -= stream.ethPerTick;
        ethStreamEndingAtTick[stream.lastTick] -= stream.ethPerTick;

        // calculate how much needs to be refunded
        uint256 ticksLeft = stream.lastTick - currentTick;
        uint256 amountToRefund = stream.ethPerTick * ticksLeft;
        (bool sent, ) = msg.sender.call{ value: amountToRefund }('');
        require(sent, 'failed to send eth');

        emit StreamCanceled(nounId, amountToRefund, ethStreamedPerTick);
    }

If the time passes more than minimumTickDuration, then the function StreamEscrow.forwardAll() can be invoked to foward all pending stream and DAO expects it will receive all ETH streams.

At the same time, assume that Alice want to cancel her stream. Because at least minimumTickDuration seconds has passed from lastForwardTimestamp, the stream of Alice for current tick should be sent to DAO.

However, Alice can front-run this transaction which is invoking StreamEscrow.forwardAll() with her transaction involving cancelStream() and don't send ethPerTick amount of ETH to DAO.

As result, Alice cancels her stream before all ETH streams are sent to DAO and DAO won't get stream it deserves.

Internal pre-conditions

No response

External pre-conditions

No response

Attack Path

No response

Impact

DAO won't get full stream as it deserves due to front-running attack with cancelStream().

PoC

No response

Mitigation

It is recomended to add StreamEscrow.forwardAll() function to StreamEscrow.cancelStream().

167 function cancelStream(uint256 nounId) public {
        require(isStreamActive(nounId), 'stream not active');

        // transfer noun to treasury
        nounsToken.transferFrom(msg.sender, nounsRecipient, nounId);

+       // fowards all pending ETH streams
+       forwardAll()

        ...
    }