0xKitsune/solstat

Vulnerability Request: Ensure Unary operations in unchecked blocks have proper overflow/underflow assertions

0xOsiris opened this issue · 0 comments

If there is a unary operation happening on two (u)ints within an unchecked code block and the result is being casted into a (u)int without sufficient overflow/underflow checks depending on the operator this is a vulnerability.

There are two obvious cases where this pattern could be generalized as a vulnerability. Should probably be noted that this vulnerability could get much more nuanced than these general cases outlined.

First: +,*,<<,** Operator no overflow check in the unchecked block.

Bad:

Contract Example {

    function leftBitwiseShift(uint128 x) public returns (uint128 z) {
        unchecked {
              z= x<<64;
        };
    }

    function multiply(uint128 x, uint128 y) public returns (uint128 z) {
        unchecked {
              z= x*y;
        };
    }
}

Good:

Contract Example {

    function leftBitwiseShift(uint128 x) public returns (uint128 z) {
        unchecked {
              z= x<<64;
              require(z<=type(uint128).max, "Overflow!");
        };
    }

    function multiply(uint128 x, uint128 y) public returns (uint128 z) {
        unchecked {
              z= x*y;
              require(z<=type(uint128).max, "Overflow!");
        };
    }
}

Second: Underflow check on - in unchecked blocks when casting result to uint
Bad:

Contract Example {
    function subtract(int128 x, int128 y) public returns (uint128 z) {
        unchecked {
              z= uint128(x-y);

        };
    }
}

Good:

Contract Example {

    function subtract(int128 x, int128 y) public returns (uint128 z) {
        unchecked {
             require(x-y>=0, "Undeflow!");
              z= uint128(x-y);

        };
    }
}