Deletion on mapping containing a structure

[code423n4]

Lines of code

https://github.com/code-423n4/2023-01-drips/blob/main/src/Drips.sol#L248

Vulnerability details

Impact

A deletion in a structure containing a mapping will not delete the mapping (see the Solidity documentation). The remaining data may be used to compromise the contract.

Proof of Concept

File: Drips.sol

    struct AmtDelta {
        /// @notice Amount delta applied on this cycle
        int128 thisCycle;
        /// @notice Amount delta applied on the next cycle
        int128 nextCycle;
    }

function _receiveDrips(uint256 userId, uint256 assetId, uint32 maxCycles)
        internal
        returns (uint128 receivedAmt)
    {
        uint32 receivableCycles;
        uint32 fromCycle;
        uint32 toCycle;
        int128 finalAmtPerCycle;
        (receivedAmt, receivableCycles, fromCycle, toCycle, finalAmtPerCycle) =
            _receiveDripsResult(userId, assetId, maxCycles);
        if (fromCycle != toCycle) {
            DripsState storage state = _dripsStorage().states[assetId][userId];
            state.nextReceivableCycle = toCycle;
            mapping(uint32 => AmtDelta) storage amtDeltas = state.amtDeltas;
            for (uint32 cycle = fromCycle; cycle < toCycle; cycle++) {
-->                delete amtDeltas[cycle];
            }
            // The next cycle delta must be relative to the last received cycle, which got zeroed.
            // In other words the next cycle delta must be an absolute value.
            if (finalAmtPerCycle != 0) {
                amtDeltas[toCycle].thisCycle += finalAmtPerCycle;
            }
        }
        emit ReceivedDrips(userId, assetId, receivedAmt, receivableCycles);
    }

Tools Used

VS Code

Recommended Mitigation Steps

Use a lock mechanism instead of a deletion to disable structure containing a mapping.

[GalloDaSballo]

Invalid

// SPDX-License-Identifier: UNLICENSED
pragma solidity 0.8.17;

import "../../lib/test.sol";
import "../../lib/Console.sol";

contract GasTest is DSTest {
    SpecificStructExample c0;

    function setUp() public {
        c0 = new SpecificStructExample();
    }
    event Debug(string name, uint128 value);
    function testGas() public {
        (uint128 thisCycle, uint128 nextCycle) = c0.removeEntry(1);
        assertEq(thisCycle, 0);
        assertEq(nextCycle, 0);
    }
}

contract SpecificStructExample {
    struct AmtDelta {
        /// @notice Amount delta applied on this cycle
        int128 thisCycle;
        /// @notice Amount delta applied on the next cycle
        int128 nextCycle;
    }

    mapping(uint32 => AmtDelta) public amtDeltas;

    constructor() {
        amtDeltas[0] = AmtDelta(123, 123);
        amtDeltas[1] = AmtDelta(123, 123);
        amtDeltas[2] = AmtDelta(123, 123);
    }

    function removeEntry(uint32 index) external returns (uint128, uint128) {
        delete amtDeltas[index];

        return (uint128(amtDeltas[index].thisCycle), uint128(amtDeltas[index].nextCycle));
    }
}

[GalloDaSballo]

[c4-judge]

GalloDaSballo marked the issue as unsatisfactory: Insufficient proof