code423n4 commented 2 years ago

Lines of code

https://github.com/RollaProject/quant-protocol/blob/main/contracts/libraries/QuantMath.sol#L73

Vulnerability details

Impact

tailing is uninitialized in QuantMath.sol/toScaledUint(). In the case here, solc-bin(0.8.13) implicitly sets uninitialized tailing stack value to 0, but this feature is undocumented and not guaranteed in solc, thus it would be best to do an explicit initialization here.

Proof of Concept

If _roundDown is True, tailing will be uninitialized

https://github.com/RollaProject/quant-protocol/blob/main/contracts/libraries/QuantMath.sol#L73

    function toScaledUint(
        FixedPointInt memory _a,
        uint256 _decimals,
        bool _roundDown
    ) internal pure returns (uint256) {
        uint256 scaledUint;

        if (_decimals == _BASE_DECIMALS) {
            scaledUint = _a.value.intToUint();
        } else if (_decimals > _BASE_DECIMALS) {
            uint256 exp = _decimals - _BASE_DECIMALS;
            scaledUint = (_a.value).intToUint() * 10**exp;
        } else {
            uint256 exp = _BASE_DECIMALS - _decimals;
            uint256 tailing;
            if (!_roundDown) {
                uint256 remainer = (_a.value).intToUint() % 10**exp;
                if (remainer > 0) tailing = 1;
            }
            scaledUint = (_a.value).intToUint() / 10**exp + tailing;
        }

        return scaledUint;
    }

Tools Used

Manual code review.

Recommended Mitigation Steps

Initialize tailing to 0 or remove tailing like the following code

    function toScaledUint(
        FixedPointInt memory _a,
        uint256 _decimals,
        bool _roundDown
    ) internal pure returns (uint256) {
        uint256 scaledUint;

        if (_decimals == _BASE_DECIMALS) {
            scaledUint = _a.value.intToUint();
        } else if (_decimals > _BASE_DECIMALS) {
            uint256 exp = _decimals - _BASE_DECIMALS;
            scaledUint = (_a.value).intToUint() * 10**exp;
        } else {
            uint256 exp = _BASE_DECIMALS - _decimals;
            scaledUint = (_a.value).intToUint()
            if (!_roundDown) {
                uint256 remainer = (_a.value).intToUint() % 10**exp;
                if (remainer > 0) scaledUint = scaledUint + 1;  
            }
        }
        return scaledUint;
    }

quantizations commented 2 years ago

As per solidity docs the default value for uint is 0: https://docs.soliditylang.org/en/v0.8.13/control-structures.html#scoping-and-declarations

alcueca commented 2 years ago

Dispute accepted, I'm yet to see an incident of an uninitialised variable not being zero.

code-423n4 / 2022-03-rolla-findings