Unsafe cast on `uniswapV3SwapCallback` can get all assets in the contract

[code423n4]

Lines of code

https://github.com/code-423n4/2023-01-numoen/blob/c75b39dc93fe327039b097ba149e1abe90d3b2f5/src/periphery/SwapHelper.sol#L38-L43

Vulnerability details

Unsafe cast on uniswapV3SwapCallback can get all assets in the contract

Summary

Type cast with overflows doesn't throw an error / revert therefore, value can be transferred out just by calling the method.

Contracts that inherit from SwapHelper:

• LendgineRouter.sol#L55

Vulnerability Detail

Even though Solidity 0.8.x is used, type casts do not throw an error. A SafeCast library must be used everywhere a typecast is done. SafeCast Reference.

Impact

Wrong values used over the code as overflow / underflow doesn't revert on cast

Proof of Concept

Underflow can occur due to unsafe cast at the following code of a int256 into uint256:

  function uniswapV3SwapCallback(int256 amount0Delta, int256 amount1Delta, bytes calldata data) external override {
    address tokenIn = abi.decode(data, (address));
    // no validation because this contract should hold no tokens between transactions

    SafeTransferLib.safeTransfer(tokenIn, msg.sender, amount0Delta > 0 ? uint256(amount0Delta) : uint256(amount1Delta));
  }

If value is sent with for example erc20 transfers to this contract, value can get drained afterwards by anybody that calls this method

Code Snippet

https://github.com/code-423n4/2023-01-numoen/blob/c75b39dc93fe327039b097ba149e1abe90d3b2f5/src/periphery/SwapHelper.sol#L38-L43

Tool used

Manual Review

Recommendation

Use safeCast library for conversions that can overflow / underflow

[kyscott18]

What would a cast to uint256 even look like? It is the highest precision. Also, check out the uniswap periphery which does the same thing https://github.com/Uniswap/v3-periphery/blob/main/contracts/SwapRouter.sol#L67

[c4-sponsor]

kyscott18 requested judge review

[berndartmueller]

The LendgineRouter contract is a periphery contract and is not meant to hold funds. Besides, there are easier ways to sweep funds out of the contract by just using the Payment utils contract functions.

[c4-judge]

berndartmueller marked the issue as unsatisfactory: Invalid