code-423n4 / 2023-01-drips-findings

0 stars 2 forks source link

UNEXPECTED DATA LOSS IS POSSIBLE DUE TO UNSAFE DOWN CASTING OF UINT40 TO UINT32 #283

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

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

Vulnerability details

Impact

In the _dripsRange function of the Drips.sol contract variable end is declared as of type uint40. After the arithmetic operations and conditional checks have been performed, the end variable is returned as uint32(end) thus down casting it unsafely.

Proof of Concept

If the uint40 end variable has a value which is more than 2^32 - 1, at the time of down casting, then the uint32(end) will truncate the most significant 8 bits of the end variable. This will trigger unwanted data loss and unexpected behaviour of the protocol.

function _dripsRange(
    DripsReceiver memory receiver,
    uint32 updateTime,
    uint32 maxEnd,
    uint32 startCap,
    uint32 endCap
) private pure returns (uint32 start, uint32 end_) {
    start = receiver.config.start();
    // slither-disable-start timestamp
    if (start == 0) {
        start = updateTime;
    }
    uint40 end = uint40(start) + receiver.config.duration();
    // slither-disable-next-line incorrect-equality
    if (end == start || end > maxEnd) {
        end = maxEnd;
    }
    if (start < startCap) {
        start = startCap;
    }
    if (end > endCap) {
        end = endCap;
    }
    if (end < start) {
        end = start;
    }
    // slither-disable-end timestamp
    return (start, uint32(end));
}

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

For example if the uint40 end variable is stored with value 2^32, which means the 33rd bit will be set to "1". Due to unsafe down casting of uint32(end) the return value will be equal to "0" since most significant 8 bits are truncated. This will return an erroneous value, thus breaking the protocol.

Similar issue can be found in the _balanceAt function of the Drips.sol contract as well. Here uint256 return value is unsafely down cast to uint128 value.

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

Tools Used

Manual and VS Code.

Recommended Mitigation Steps

Import Openzeppelin safeCast.sol library into the contract and use its toUint32() function to safely down cast to uint32(end).

GalloDaSballo commented 1 year ago

Invalid, the 2 values are u32, meaning they will not be able to overflow the value (they are too small)

Screenshot 2023-02-06 at 17 17 35
c4-judge commented 1 year ago

GalloDaSballo marked the issue as unsatisfactory: Invalid