Core function of `Ocean` contract like `doInteraction | doMultipleInteractions | forwardedDoInteraction` may revert under certain conditions

[c4-bot-6]

Lines of code

https://github.com/code-423n4/2023-11-shellprotocol/blob/485de7383cdf88284ee6bcf2926fb7c19e9fb257/src/adapters/OceanAdapter.sol#L150-L153

Vulnerability details

Impact

Core functions like doInteraction(), forwardedDoInteraction(), doMultipleInteractions(), forwardedDoMultipleInteractions() etc, will always revert under certain conditions due to overflow in calculations.

Proof of Concept

Core external functions like doInteraction will call the internal _executeInteraction function. This function will call _computeOutputAmount internal function that calls computeOutputAmount

IOceanPrimitive(primitive).computeOutputAmount(inputToken, outputToken, inputAmount, userAddress, metadata);

This computeOutputAmount is protected by onlyOcean modifier, so no one can call it directly.

function computeOutputAmount(...)
        external
        override
        onlyOcean
        returns (uint256 outputAmount)
    {
...
        outputAmount = primitiveOutputAmount(inputToken, outputToken, unwrappedAmount, metadata);
...
    }

primitiveOutputAmount function is virtual and overridden in Curve2PoolAdapter and CurveTricryptoAdapter Both implementation has same outputAmount calculation:

outputAmount = _convertDecimals(decimals[outputToken], NORMALIZED_DECIMALS, rawOutputAmount);

NORMALIZED_DECIMALS is a constant variable with value of 18. _convertDecimals will overflow and revert() if rawOutputAmount will be bigger than type(uint256).max / 10 ** (NORMALIZED_DECIMALS - decimals[outputToken]) An issue in this lines:

} else if (decimalsFrom < decimalsTo) {
    uint256 shift = 10 ** (uint256(decimalsTo - decimalsFrom));
    convertedAmount = amountToConvert * shift;

if amountToConvert is large enough multiplication will overflow. PoC Assume that outputToken is UDST which has 6 decimals.

Then uint256 shift = 10 (uint256(18 - 6)) == 10 12, so amountToConvert should be greater than type(uint256).max / 10 ** 12 to overflow.

function testOceanAdapterConvertDecimalsOverflow() public {
    uint8 decimalsFrom = 6;
    uint8 decimalsTo = 18;
    uint256 amountToConvert = type(uint256).max / 10 ** 12 + 1;

    _convertDecimals(decimalsFrom, decimalsTo, amountToConvert);
}

Running 1 test for src/test/fork/TestOceanAdapter.t.sol:TestOceanAdapter
[FAIL. Reason: panic: arithmetic underflow or overflow (0x11)] testOceanAdapterConvertDecimalsOverflow() (gas: 965)
Test result: FAILED. 0 passed; 1 failed; 0 skipped; finished in 3.24ms

Tools Used

foundry

Recommended Mitigation Steps

Limit Input Ranges of input values for amountToConvert.

This can involve setting upper limits on these values to ensure that amount do not lead to overflows.

Assessed type

Under/Overflow

[c4-pre-sort]

raymondfam marked the issue as insufficient quality report

[c4-pre-sort]

raymondfam marked the issue as duplicate of #14

[c4-judge]

0xA5DF marked the issue as unsatisfactory: Invalid