code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-09-canto/blob/65fbb8b9de22cf8f8f3d742b38b4be41ee35c468/src/Swap/BaseV1-core.sol#L598-L611

Vulnerability details

Impact

Stable and non-stable pair use different formula to calculate the invariant k_. If a non-stable pair is treated as stable, or vice versa.

$x^3y+y^3x$ behave quite differently compare with $xy$, on the edge of relative stable price range, price volatility could be abnormally high. Swap amounts could loss accuracy and be abused. The following results might happen:

careless users might loss fund due to large slippage
high slippage may result in lots of swap failure, affecting the normal contract functionality
malicious users have more motivation to perform sandwich attack with much more profits
abnormal arbitrage opportunity due to too slow/fast price movement, attracting arbitrage bots, discouraging real users
easier oracle manipulation on the edging interval, further affecting lending function, even big fund loss

Proof of Concept

Stable pair parameter is input by the user, and no checks on if it is true. src/Swap/BaseV1-core.sol

    function createPair(address tokenA, address tokenB, bool stable) external returns (address pair) {
        require(tokenA != tokenB, "IA"); // BaseV1: IDENTICAL_ADDRESSES
        (address token0, address token1) = tokenA < tokenB ? (tokenA, tokenB) : (tokenB, tokenA);
        require(token0 != address(0), "ZA"); // BaseV1: ZERO_ADDRESS
        require(getPair[token0][token1][stable] == address(0), "PE"); // BaseV1: PAIR_EXISTS - single check is sufficient
        bytes32 salt = keccak256(abi.encodePacked(token0, token1, stable)); // notice salt includes stable as well, 3 parameters
        (_temp0, _temp1, _temp) = (token0, token1, stable);
        pair = address(new BaseV1Pair{salt:salt}());
        getPair[token0][token1][stable] = pair;
        getPair[token1][token0][stable] = pair; // populate mapping in the reverse direction
        allPairs.push(pair);
        isPair[pair] = true;
        emit PairCreated(token0, token1, stable, pair, allPairs.length);
    }

A mistaken input can happen, even worse, malicious user can do it on purpose to take advantage of the k_ calculation.

Tools Used

Manual analysis.

Recommended Mitigation Steps

Use a whitelist for stablecoins, only allow stable pair creation if both tokens are in the whitelist.

nivasan1 commented 2 years ago

There is a whitelist on what stable pairs are referenced by the lending market when determining the prices of assets (setStable), this method is only accessible through chain governance. As the getUnderlyingPrice method is specifically designed to be used by the Comptroller, a whitelist on the factory is not needed

0xean commented 2 years ago

closing as invalid.

code-423n4 / 2022-09-canto-findings