Vulnerability where users could withdraw arbitrary tokens from the pool that they did not provide liquidity for

[c4-bot-2]

Lines of code

https://github.com/code-423n4/2023-11-shellprotocol/blob/485de7383cdf88284ee6bcf2926fb7c19e9fb257/src/adapters/CurveTricryptoAdapter.sol#L264-L292 https://github.com/code-423n4/2023-11-shellprotocol/blob/485de7383cdf88284ee6bcf2926fb7c19e9fb257/src/adapters/CurveTricryptoAdapter.sol#L214-L221

Vulnerability details

Impact

Users could drain other tokens from the pool that they did not originally provide as liquidity. This could lead to loss of funds for liquidity providers in the pool. The severity is high because it allows arbitrary tokens to be withdrawn without actually removing the corresponding liquidity. This breaks the expectations of liquidity providers and undermines the integrity of the pool

Proof of Concept

There is a vulnerability where users could withdraw arbitrary tokens from the pool that they did not provide liquidity for. Here is a detailed explanation: The _determineComputeType() function is used to decide if a transaction is a swap, deposit, or withdraw. For a withdraw, it simply checks if the input token is the lpTokenId and the output is xToken, yToken or zToken. There is no check that liquidity was actually provided for that specific token. For example, a user could:

1. Deposit 1000 DAI and get LP tokens

2. Call withdraw setting: • inputToken = lpTokenId • outputToken = wBTC This would be treated as a valid withdraw, even though they never provided wBTC liquidity. Here is the relevant code:

   function _determineComputeType( uint256 inputToken, uint256 outputToken ) private view returns (ComputeType computeType) { if (inputToken == lpTokenId) && (outputToken == xToken || outputToken == yToken || outputToken == zToken)) { return ComputeType.Withdraw; } }

When ComputeType is Withdraw, it calls remove_liquidity_one_coin which allows withdrawing any token regardless of what was deposited:

   ICurveTricrypto(primitive).remove_liquidity_one_coin(rawInputAmount, 
   indexOfOutputAmount, 0);

Tools Used

Manual

Recommended Mitigation Steps

The contract could track which tokens had liquidity added. Then on withdraw, check that the outputToken matches one of those tokens. A suggestive example, add a mapping:

   mapping(uint256 => bool) hasLiquidity;

Set hasLiquidity to true when depositing each token. Then modify _determineComputeType :

   if (hasLiquidity[outputToken] && 
       inputToken == lpTokenId && 
       (outputToken == xToken || outputToken == yToken || outputToken == zToken)) {
       return ComputeType.Withdraw
   }

This would prevent withdrawing arbitrary tokens that liquidity was not provided for.

Assessed type

Other

[c4-pre-sort]

raymondfam marked the issue as insufficient quality report

[c4-pre-sort]

raymondfam marked the issue as duplicate of #20

[c4-judge]

0xA5DF changed the severity to QA (Quality Assurance)

[c4-judge]

0xA5DF marked the issue as grade-c

[0xA5DF]

Low quantity