code-423n4 / 2023-11-shellprotocol-findings

7 stars 7 forks source link

Missing Fallback Function in Curve2PoolAdapter Smart Contract #199

Closed c4-bot-2 closed 10 months ago

c4-bot-2 commented 10 months ago

Lines of code

https://github.com/code-423n4/2023-11-shellprotocol/blob/main/src/adapters/Curve2PoolAdapter.sol#L20 https://github.com/code-423n4/2023-11-shellprotocol/blob/main/src/adapters/CurveTricryptoAdapter.sol#L291

Vulnerability details

Impact

The absence of a fallback function in a Curve2PoolAdapter.sol contract can lead to unexpected behavior and potential loss of funds. If Ether is sent to this contract without a function call and the contract does not have a fallback function, the transaction will fail and all gas will be consumed. This could lead to financial loss for the user and potential denial of service.

Proof of Concept

It can be observed that Curve2PoolAdapter.sol contract does not have a fall back function why CurveTricryptoAdapter.sol has it i.e

contract CurveTricryptoAdapter is OceanAdapter {
  ...
    fallback() external payable { }
}

It can be noted that the reason why CurveTricryptoAdapter.sol probably does not have a fallback function is because it does not deal directly with the native token or ether. However, it’s still a good practice to include a fallback function to handle unexpected situations. Ocean should include a fallback function that reverts transactions, effectively preventing the contract from accepting Ether and also prevent locked funds since there is no way to transfer it out.

Tools Used

Manual Review, Remix

Recommended Mitigation Steps

A reversion Fallback should be added to the Curve2PoolAdapter contract since it should not receive Ether

contract Curve2PoolAdapter is OceanAdapter {
  ...
    fallback() external {
        revert("This contract does not accept Ether");
    }
}

Assessed type

Payable

c4-pre-sort commented 10 months ago

raymondfam marked the issue as insufficient quality report

c4-pre-sort commented 10 months ago

raymondfam marked the issue as primary issue

raymondfam commented 10 months ago

No ETH involved. Only USDC/USDT.

https://github.com/code-423n4/2023-11-shellprotocol/blob/main/src/adapters/Curve2PoolAdapter.sol#L16-L19

/**
 * @notice
 *   curve2pool adapter contract enabling swapping, adding liquidity & removing liquidity for the curve usdc-usdt pool
 */
c4-judge commented 10 months ago

0xA5DF marked the issue as unsatisfactory: Invalid