088.md

Perfect Yellow Fox

High

Incorrect Stream ethperTick update in the StreamEscrow::createStream.

Summary In the createStream function that allows potential overwriting of existing stream data, compromising the contract's stream management integrity. code Vulnerability Details

The createStream function contains a flaw that allows incorrect overwriting of existing stream data for a specific Noun token. This issue occurs when the function is called a second time before isStreamActive is set to true. Instead of updating streams[nounId] stream with the correct newEthStreamedPerTick value (calculated as ethStreamedPerTick from the past tick record plus ethPerTick from the new tick record), the function mistakenly uses only the ethPerTick value from the new tick record. This error compromises the integrity of the contract’s stream management.

        function createStream(uint256 nounId, uint16 streamLengthInTicks) public payable {
        require(allowedToCreateStream[msg.sender], 'not allowed');
        require(isApprovedOrOwner(msg.sender, nounId), 'only noun owner or approved');
        require(!isStreamActive(nounId), 'stream active');

        // register new stream
        uint128 ethPerTick = toUint128(msg.value / streamLengthInTicks);
        uint32 streamLastTick = currentTick + streamLengthInTicks;
        ethStreamEndingAtTick[streamLastTick] += ethPerTick;

        // the remainder is immediately streamed to the DAO
        uint256 remainder = msg.value % streamLengthInTicks;
        sendETHToTreasury(remainder);

        uint128 newEthStreamedPerTick = ethStreamedPerTick + ethPerTick;
        ethStreamedPerTick = newEthStreamedPerTick;
       
@>        streams[nounId] = Stream({ ethPerTick: ethPerTick, canceled: false, lastTick: streamLastTick });
        emit StreamCreated(nounId, msg.value, streamLengthInTicks, ethPerTick, newEthStreamedPerTick, streamLastTick);
    }

Impact This vulnerability can result in unexpected stream data manipulation, potential loss of funds, and compromised stream accounting for Noun token streams.

Proof of Concept The scenario before portray how vulnerability can occur:

1. Initial Setup:

   • Assume streams[nounId] already has an existing stream record.
   • Ensure the isStreamActive flag is false for the specific Noun token.

2. First Call to createStream:

   • Call the createStream function to initialize a new stream.
   • Set ethPerTick for the first tick (e.g., 100 wei).

3. Second Call to createStream Before isStreamActive Becomes true: Call the createStream function again for the same nounId. Provide a new ethPerTick value (e.g., 50 wei).

4. Expected Behavior:

   • The streams[nounId] should be updated with newEthStreamedPerTick, calculated as:

     newEthStreamedPerTick = ethStreamedPerTick (previous) + ethPerTick (current)

   • For example:

     newEthStreamedPerTick = 100 + 50 = 150 wei

5. Actual Behavior: - The function overwrites the existing streams[nounId] with only the ethPerTick value from the new record (50 wei). - The previously streamed value (100 wei) is discarded. Code demostration: ```javascript // Pseudocode Example createStream(nounId, ethPerTick: 100); // First call // streams[nounId].ethStreamedPerTick = 100

      createStream(nounId, ethPerTick: 50); // Second call
      // `streams[nounId].ethStreamedPerTick` = 50 (incorrect overwrite)
    ```
   

Recommended Mitigation

        function createStream(uint256 nounId, uint16 streamLengthInTicks) public payable {
        require(allowedToCreateStream[msg.sender], 'not allowed');
        require(isApprovedOrOwner(msg.sender, nounId), 'only noun owner or approved');
        require(!isStreamActive(nounId), 'stream active');

        // register new stream
        uint128 ethPerTick = toUint128(msg.value / streamLengthInTicks);
        uint32 streamLastTick = currentTick + streamLengthInTicks;
        ethStreamEndingAtTick[streamLastTick] += ethPerTick;

        // the remainder is immediately streamed to the DAO
        uint256 remainder = msg.value % streamLengthInTicks;
        sendETHToTreasury(remainder);

        uint128 newEthStreamedPerTick = ethStreamedPerTick + ethPerTick;
        ethStreamedPerTick = newEthStreamedPerTick;
       
-        streams[nounId] = Stream({ ethPerTick: ethPerTick, canceled: false, lastTick: streamLastTick });
+        streams[nounId] = Stream({ ethPerTick: ethStreamedPerTick, canceled: false, lastTick: streamLastTick });
        emit StreamCreated(nounId, msg.value, streamLengthInTicks, ethPerTick, newEthStreamedPerTick, streamLastTick);
    }