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

7 stars 7 forks source link

Incorrect indexing in constructor of Curve Adapters causes both contracts to assume xToken and lpToken to be the same token #308

Closed c4-bot-5 closed 8 months ago

c4-bot-5 commented 8 months ago

Lines of code

https://github.com/code-423n4/2023-11-shellprotocol/blob/485de7383cdf88284ee6bcf2926fb7c19e9fb257/src/adapters/Curve2PoolAdapter.sol#L77-L95 https://github.com/code-423n4/2023-11-shellprotocol/blob/485de7383cdf88284ee6bcf2926fb7c19e9fb257/src/adapters/Curve2PoolAdapter.sol#L65 https://github.com/code-423n4/2023-11-shellprotocol/blob/485de7383cdf88284ee6bcf2926fb7c19e9fb257/src/adapters/CurveTricryptoAdapter.sol#L73 https://github.com/code-423n4/2023-11-shellprotocol/blob/485de7383cdf88284ee6bcf2926fb7c19e9fb257/src/adapters/CurveTricryptoAdapter.sol#L85-L111

Vulnerability details

Impact

Curve Adapter contracts are unusable, as the protocol won't be able to tell the difference between which token (USDC/USDT or lpToken) is intended on being used for the transaction.

Vulnerability details

In the constructors of the Curve2PoolAdapter.sol and the CurveTricryptoAdapter.sol the protocol indexes the yToken and the zToken with 1 and 2 respectively in the indexOf mapping.

https://github.com/code-423n4/2023-11-shellprotocol/blob/485de7383cdf88284ee6bcf2926fb7c19e9fb257/src/adapters/Curve2PoolAdapter.sol#L65 https://github.com/code-423n4/2023-11-shellprotocol/blob/485de7383cdf88284ee6bcf2926fb7c19e9fb257/src/adapters/CurveTricryptoAdapter.sol#L73

Curve2PoolAdapter.sol
    mapping(uint256 => int128) indexOf;

CurveTricryptoAdapter.sol
    mapping(uint256 => uint256) indexOf;

However, in both contracts the protocol fails to index the xToken and the liquidity token resulting in both tokens automatically assigned the value 0 in the indexOf mapping. By allowing this to happen, anytime the primitiveOutputAmount() function of the Curve Adapters are called the contract will always assume that the xToken(USDC/USDT) and the liquidity token are the same token.

Constructors of both contracts indexing tokens: https://github.com/code-423n4/2023-11-shellprotocol/blob/485de7383cdf88284ee6bcf2926fb7c19e9fb257/src/adapters/Curve2PoolAdapter.sol#L77-L95 https://github.com/code-423n4/2023-11-shellprotocol/blob/485de7383cdf88284ee6bcf2926fb7c19e9fb257/src/adapters/CurveTricryptoAdapter.sol#L85-L111

Curve2PoolAdapter.sol
    constructor(address ocean_, address primitive_) OceanAdapter(ocean_, primitive_) {
        address xTokenAddress = ICurve2Pool(primitive).coins(0);
        xToken = _calculateOceanId(xTokenAddress, 0);
        underlying[xToken] = xTokenAddress;
        decimals[xToken] = IERC20Metadata(xTokenAddress).decimals();
        _approveToken(xTokenAddress);

        address yTokenAddress = ICurve2Pool(primitive).coins(1);
        yToken = _calculateOceanId(yTokenAddress, 0);
        indexOf[yToken] = int128(1);
        underlying[yToken] = yTokenAddress;
        decimals[yToken] = IERC20Metadata(yTokenAddress).decimals();
        _approveToken(yTokenAddress);

        lpTokenId = _calculateOceanId(primitive_, 0);
        underlying[lpTokenId] = primitive_;
        decimals[lpTokenId] = IERC20Metadata(primitive_).decimals();
        _approveToken(primitive_);
    }
CurveTricryptoAdapter.sol
    constructor(address ocean_, address primitive_) OceanAdapter(ocean_, primitive_) {
        address xTokenAddress = ICurveTricrypto(primitive).coins(0);
        xToken = _calculateOceanId(xTokenAddress, 0);
        underlying[xToken] = xTokenAddress;
        decimals[xToken] = IERC20Metadata(xTokenAddress).decimals();
        _approveToken(xTokenAddress);

        address yTokenAddress = ICurveTricrypto(primitive).coins(1);
        yToken = _calculateOceanId(yTokenAddress, 0);
        indexOf[yToken] = 1;
        underlying[yToken] = yTokenAddress;
        decimals[yToken] = IERC20Metadata(yTokenAddress).decimals();
        _approveToken(yTokenAddress);

        address wethAddress = ICurveTricrypto(primitive).coins(2);
        zToken = _calculateOceanId(address(0x4574686572), 0); // 
        hexadecimal(ascii("Ether"))
        indexOf[zToken] = 2;
        underlying[zToken] = wethAddress;
        decimals[zToken] = NORMALIZED_DECIMALS;
        _approveToken(wethAddress);

        address lpTokenAddress = ICurveTricrypto(primitive).token();
        lpTokenId = _calculateOceanId(lpTokenAddress, 0);
        underlying[lpTokenId] = lpTokenAddress;
        decimals[lpTokenId] = IERC20Metadata(lpTokenAddress).decimals();
        _approveToken(lpTokenAddress);
    }

Proof of Concept

To assert that the xToken and lpToken of both contracts assume the value of 0, will create 2 new test files with the same setup used for the Curve adapters contracts. First we will add some new variables under the indexOf mapping to the Curve adapters contracts to store the index value from the mapping so it can be read in the test file.

Curve2Pooladapter.sol
    int128 public xt;
    int128 public yt;
    int128 public lt;

CurveTricryptoAdapter.sol
    uint256 public xt;
    uint256 public yt;
    uint256 public zt;
    uint256 public lt;

Then we assign them the index values in the constructor.

Curve2PoolAdapter.sol
   xt = indexOf[xToken];
   yt = indexOf[yToken];
   lt = indexOf[lpTokenId];

CurveTricryptoAdapter.sol
   xt = indexOf[xToken];
   yt = indexOf[yToken];
   zt = indexOf[zToken];
   lt = indexOf[lpTokenId];

In the test files we create a function that asserts their values given in the constructors

    function testToken() public {
        assertEq(adapter.yt(), int128(1));
        assertEq(adapter.xt(), 0);
        assertEq(adapter.lt(), 0);
    }
    function testTricryptoTokens() public {
        assertEq(adapter.yt(), 1);
        assertEq(adapter.zt(), 2);
        assertEq(adapter.xt(), 0);
        assertEq(adapter.lt(), 0);
    }

Tools Used

Manual review

Recommended Mitigation Steps

Index the lpTokenId as int128(2) in the Curve2PoolAdapter.sol contract and as 3 in the CurveTricryptoAdapter.sol contracts

Curve2PoolAdapter.sol  constructor()
        lpTokenId = _calculateOceanId(primitive_, 0);
+        indexOf[lpTokenId] = int128(2);
        underlying[lpTokenId] = primitive_;
        decimals[lpTokenId] = IERC20Metadata(primitive_).decimals();
        _approveToken(primitive_);

CurveTricryptoAdapter.sol  constructor()
        address lpTokenAddress = ICurveTricrypto(primitive).token();
        lpTokenId = _calculateOceanId(lpTokenAddress, 0);
+        indexOf[lpTokenId] = 3;
        underlying[lpTokenId] = lpTokenAddress;
        decimals[lpTokenId] = IERC20Metadata(lpTokenAddress).decimals();
        _approveToken(lpTokenAddress);

## Assessed type

ERC20
c4-pre-sort commented 8 months ago

raymondfam marked the issue as insufficient quality report

c4-pre-sort commented 8 months ago

raymondfam marked the issue as primary issue

raymondfam commented 8 months ago

It's 0 by default then.

c4-judge commented 8 months ago

0xA5DF marked the issue as unsatisfactory: Invalid

0xA5DF commented 8 months ago

Agree with Raymond, everything works as intended