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

7 stars 7 forks source link

Invalid WETH inclusion for Curve's Tricrypto pools. #250

Closed c4-bot-10 closed 9 months ago

c4-bot-10 commented 9 months ago

Lines of code

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

Vulnerability details

Impact

For some of Curve's Tricrypto pools, CurveTricryptoAdapter will not be working.

Proof of Concept

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);

In CurveTricryptoAdapter contract, it assumes that the last token of tricrypto pools are one of WETH smart contracts so it just overwrites token data with Ocean's predefined token id for WETH to avoid duplication of different WETH tokens. However, some of Curve's Tricrypto pools do not include WETH in their token list including TryLSD(wstETH + rETH + sfrxETH) and TricrpytoLLAMA(crvUSD + tBTC + wstETH).

As a result, CurveTricryptoAdapter's logic like Deposit, Swap, and Withdraw does not work because it expectes ETH but actually other LSTs are processed.

Tools Used

Manual Review

Recommended Mitigation Steps

Check if last token of Curve's Tricrypto pool is one of WETH tokens, if it is, follow current logic which sets use_eth to True, otherwise, set use_eth to False.

Assessed type

Context

c4-pre-sort commented 9 months ago

raymondfam marked the issue as sufficient quality report

c4-pre-sort commented 9 months ago

raymondfam marked the issue as duplicate of #185

c4-judge commented 9 months ago

0xA5DF marked the issue as not a duplicate

0xA5DF commented 9 months ago

Hey @viraj124 This one as well if possible, similar to #59 but seems to talk about a different issue

viraj124 commented 9 months ago

@0xA5DF the pool will always use the weth token only that is a given so we can close I think

c4-sponsor commented 9 months ago

viraj124 (sponsor) disputed

0xA5DF commented 9 months ago

Thanks! I agree with the sponsor, this is documented at the top of the contract as well:

/**
 * @notice
 *   curve tricrypto adapter contract enabling swapping, adding liquidity & removing liquidity for the curve usdt-wbtc-eth pool
 */
c4-judge commented 9 months ago

0xA5DF marked the issue as unsatisfactory: Invalid

c4-judge commented 9 months ago

0xA5DF removed the grade

c4-judge commented 9 months ago

0xA5DF marked the issue as unsatisfactory: Invalid