069.md

Mysterious Onyx Butterfly

Medium

Missing Validation for streamLengthInTicks in createStream Function

Summary

The lack of validation for streamLengthInTicks in the createStream function can cause a division-by-zero error or an ineffective stream configuration. This can lead to transaction reverts or the creation of streams with no meaningful effect for users if improper inputs are provided.

Root Cause

In the StreamEscrow.sol

In the createStream function uint128 ethPerTick = toUint128(msg.value / streamLengthInTicks); Missing Validation: The function does not check if streamLengthInTicks is greater than zero or if msg.value is sufficient to divide evenly across streamLengthInTicks.

Internal pre-conditions

Internal Pre-Conditions The allowedToCreateStream[msg.sender] mapping must allow the caller. The msg.sender must be approved or the owner of the nounId NFT. The nounId must not have an active stream (isStreamActive(nounId)).

External pre-conditions

The streamLengthInTicks input is set to zero or a value greater than msg.value in the transaction. No prior validation is done by the caller or any external protocol interacting with this contract.

Attack Path

A trusted address calls the createStream function with: streamLengthInTicks = 0: This causes a division-by-zero error, resulting in a transaction revert. streamLengthInTicks > msg.value: This causes ethPerTick to be zero, rendering the stream ineffective and sending the remainder of msg.value directly to the DAO. The stream is either not created or becomes non-functional, undermining its intended purpose.

Impact

For streamLengthInTicks = 0: Users experience transaction reverts, wasting gas fees. For streamLengthInTicks > msg.value: The stream is created but ineffective (ethPerTick = 0), preventing any meaningful streaming of ETH. The remaining funds are lump-summed to the DAO, bypassing the stream mechanism.

PoC

// Example test for a zero or insufficient streamLengthInTicks function testCreateStreamFailsOnInvalidStreamLengthInTicks() public { uint256 invalidStreamLength = 0; // Or set it greater than msg.value uint256 msgValue = 1 ether; uint256 nounId = 1;

// Expect revert on invalid streamLengthInTicks
vm.expectRevert("streamLengthInTicks must be greater than zero");
streamEscrow.createStream{value: msgValue}(nounId, invalidStreamLength);

}

Mitigation

Add Validation for streamLengthInTicks in createStream: require(streamLengthInTicks > 0, "streamLengthInTicks must be greater than zero"); require(msg.value >= streamLengthInTicks, "msg.value must be >= streamLengthInTicks"); Ensure Defense-in-Depth: Validate streamLengthInTicks during the parameter-setting phase (setStreamEscrowParams). Double-check the value within the createStream function for robustness.