search

code-423n4 / 2021-11-vader-findings

0 stars 0 forks source link

Wrong design of `swap()` results in unexpected and unfavorable outputs #213

Open code423n4 opened 3 years ago

code423n4 commented 3 years ago

Handle

WatchPug

Vulnerability details

The current formula to calculate the amountOut for a swap is:

https://github.com/code-423n4/2021-11-vader/blob/429970427b4dc65e37808d7116b9de27e395ce0c/contracts/dex/math/VaderMath.sol#L99-L111

function calculateSwap(
    uint256 amountIn,
    uint256 reserveIn,
    uint256 reserveOut
) public pure returns (uint256 amountOut) {
    // x * Y * X
    uint256 numerator = amountIn * reserveIn * reserveOut;

    // (x + X) ^ 2
    uint256 denominator = pow(amountIn + reserveIn);

    amountOut = numerator / denominator;
}

We believe the design (the formula) is wrong and it will result in unexpected and unfavorable outputs.

Specifically, if the amountIn is larger than the reserveIn, the amountOut starts to decrease.

PoC

Given:

  1. If Alice swap 2 BTC for USDV, will get 50000 USDV as output;
  2. If Bob swap 2.1 BTC for USDV, will only get 49970.25 USDV as output;
  3. If Carol swap 2.2 BTC for USDV, will only get 49886.62 USDV as output.

For the same pool reserves, paying more for less output token is unexpected and unfavorable.

SamSteinGG commented 3 years ago

This is the intended design of the Thorchain CLP model. Can the warden provide a tangible attack vector in the form of a test?

alcueca commented 2 years ago

It is true that the effect will be surprising to the user, and the issue is acknowledged by the sponsor.

SamSteinGG commented 2 years ago

@alcueca We do not acknowledge the issue. This is the intended design of the CLP model and the amount supplied for a trade is meant to be safeguarded off-chain. It is an inherent trait of the model.