code-423n4 / 2023-06-canto-findings

1 stars 0 forks source link

Uniswap k invariant is not checked #38

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-06-canto/blob/a4ff2fd2e67e77e36528fad99f9d88149a5e8532/Canto/x/coinswap/keeper/swap.go#L11-L27

Vulnerability details

Impact

The core swap logic is the implementation of uniswapv2's AMM invariants, calculate the input and output by k, but not checking k value after swap, which may affect the balance of the pool.
If the actual number of tokens transferred is not the calculated number, it will cause the pool k value to decrease and eventually run out of funds.
Considering the fee-on-transfer tokens, the number of tokens transferred to the pool is lower than expected, resulting in a decrease in k value and affecting the pool balance.
It is not clear whether whitelisted tokens charge fee or will be charged and support other tokens in the future, but it has a great impact on pool, so marked as med and wait the judge for decision.

Proof of Concept

A general process of TokenA -> Canto:

  1. The pool currently has TokenA:Canto = 100:100
  2. The user transfers a large amount of TokenA from another chain and will swap for 4 Cantos
  3. Calculate the TokenA to be passed as 100 - 100 * 100 / 104 + 1
  4. TokenA is transferred to the pool and Canto is transferred out, but because TokenA has other logic, such as charging fee, the actual amount transferred is less than the calculated amount
  5. Users use a small amount to arbitrage canto that exceeds the actual amount
  6. At this time, the quantity of Canto in the pool is less than TokenA, and the price of Canto is higher than the market. Arbitrageurs sell Canto and get excess TokenA
  7. Repeat the above process again. Although only 4 Cantos can be exchanged each time, the pool will be exhausted due to the large user base

Tools Used

Manual review

Recommended Mitigation Steps

Check k invariant after swap

Assessed type

Uniswap

c4-pre-sort commented 1 year ago

JeffCX marked the issue as low quality report

JeffCX commented 1 year ago

Vague report, Insufficient quality and proof

c4-judge commented 1 year ago

0xean changed the severity to QA (Quality Assurance)

c4-judge commented 1 year ago

0xean marked the issue as grade-c