pinalikefruit - Flaw in `GSPFunding::buyShare()` function causes user fund losses

[sherlock-admin2]

pinalikefruit

high

Flaw in GSPFunding::buyShare() function causes user fund losses

Summary

A critical flaw in the buyShare() function leads to substantial financial losses for users. This issue stems from an improper calculation method for determining share allocation.

Vulnerability Detail

The vulnerability occurs during the share purchasing process, which involves the following steps:

1. The user transfers funds to the contract.
2. The user executesbuyShares().

The core issue lies in how the buyShares() function calculates the number of shares to allocate. It bases the calculation on the smaller amount of the two tokens deposited, leading to an inconsistent and detrimental distribution of shares. This results in scenarios where users receive the same number of shares regardless of the actual amount deposited.

Result of PoC:

 ---------------------------------------------
  Status Before Executing 'buyShares'
  ---------------------------------------------
  USER_A: 
  - Share balance:  0
  - USDC balance:  1e6
  - DAI balance:  1e18
  ---------------------------------------------
  USER_B: 
  - Share balance:  0
  - USDC balance:  10,000e6
  - DAI balance:  1e18
  ---------------------------------------------
  Status After Executing 'buyShares'
  ---------------------------------------------
  USER_A: 
  - Share balance:  1,000,000,000,000,000,000
  ---------------------------------------------
  USER_B: 
  - Share balance:  1,000,000,000,000,000,000
  ---------------------------------------------

As illustrated, both USER_A and USER_B receive the same share balance post-transaction, despite their significantly different initial balances. Consequently, this leads to an inequitable share distribution, notably disadvantaging USER_B whose additional funds become irretrievably blocked in the contract.

Proof of Concept (PoC)

The Solidity script below demonstrates this vulnerability. The test scenario involves two users (USER_A and USER_B) buying shares with different amounts of deposited funds but ending up with an identical share balance.

// SPDX-License-Identifier: MIT 

pragma solidity 0.8.16;
import "forge-std/console.sol";
import { Test } from "forge-std/Test.sol";
import { DeployGSP } from "../scripts/DeployGSP.s.sol";
import { GSP } from "../contracts/GasSavingPool/impl/GSP.sol";
import { IERC20 } from "@openzeppelin/contracts/token/ERC20/IERC20.sol";

contract BadCalculationOfShare is Test {
    GSP gsp;

    address DODO_TEAM = vm.addr(1);
    address USER_A = vm.addr(2);
    address USER_B = vm.addr(3);

    address constant USDC_WHALE = 0x51eDF02152EBfb338e03E30d65C15fBf06cc9ECC;
    address constant DAI_WHALE = 0x25B313158Ce11080524DcA0fD01141EeD5f94b81;
    address constant USDC = 0xA0b86991c6218b36c1d19D4a2e9Eb0cE3606eB48; // 6 decimal
    address constant DAI = 0x6B175474E89094C44Da98b954EedeAC495271d0F; // 18 decimal
    IERC20 private usdc = IERC20(USDC);
    IERC20 private dai = IERC20(DAI);

    // Test Params
    uint256 constant BASE_RESERVE = 10000e18; // 10,000 DAI
    uint256 constant QUOTE_RESERVE = 10000e6; // 10,000 USDC

    uint256 constant BASE_INPUT = 1e18; // 1 DAI
    uint256 constant QUOTE_INPUT = 1e6; // 1 USDC 

    uint256 constant QUOTE_LARGE = 10000e6; // 10,000 USDC

    function setUp() public {
        // Create and deploy GSP.sol
        DeployGSP deployGSP = new DeployGSP();
        gsp = deployGSP.run();

        // Transfer amoutn of DAI and USDC to DODO_TEAM
        vm.startPrank(DAI_WHALE);
        dai.transfer(DODO_TEAM, BASE_RESERVE);
        dai.transfer(USER_A, BASE_INPUT);
        dai.transfer(USER_B, BASE_INPUT);
        vm.stopPrank();

        vm.startPrank(USDC_WHALE);
        usdc.transfer(DODO_TEAM,QUOTE_RESERVE);
        usdc.transfer(USER_A,QUOTE_INPUT);
        usdc.transfer(USER_B,QUOTE_LARGE);
        vm.stopPrank();
    }

    function test_lostQuoteToken() public { 
        console.log("---------------------------------------------");
        console.log("Before the Buy Share");
        console.log("---------------------------------------------");
        console.log("USER_A share balance: ", gsp.balanceOf(USER_A));
        console.log("USER_A USDC balance: ", usdc.balanceOf(USER_A));
        console.log("USER_A DAI balance: ", dai.balanceOf(USER_A));
        console.log("---------------------------------------------");
        console.log("USER_B share balance: ", gsp.balanceOf(USER_B));
        console.log("USER_B USDC balance: ", usdc.balanceOf(USER_B));
        console.log("USER_B DAI balance: ", dai.balanceOf(USER_B));
        // DODO TEAM made a transfer 
        vm.startPrank(DODO_TEAM);
        dai.transfer(address(gsp), BASE_RESERVE);
        usdc.transfer(address(gsp), QUOTE_RESERVE);
        gsp.buyShares(DODO_TEAM);
        uint256 totalSupplyBefore = gsp.totalSupply();
        vm.stopPrank();

       // User A buy share
        vm.startPrank(USER_A);
        dai.transfer(address(gsp), BASE_INPUT);
        usdc.transfer(address(gsp), QUOTE_INPUT);
        gsp.buyShares(USER_A);
        uint shareUserA = gsp.balanceOf(USER_A);
        gsp.sellShares(shareUserA,USER_A,0,0,"",block.timestamp + 100);
        vm.stopPrank();
        uint256 totalSupplyAfter = gsp.totalSupply();
        assertEq(totalSupplyAfter,totalSupplyBefore);

        // User B buy share
        vm.startPrank(USER_B);
        dai.transfer(address(gsp), BASE_INPUT);
        usdc.transfer(address(gsp), QUOTE_LARGE);
        gsp.buyShares(USER_B);
        uint shareUserB = gsp.balanceOf(USER_B);
        vm.stopPrank();
        assertEq(shareUserA,shareUserB);

        console.log("---------------------------------------------");
        console.log("After share was buy");
        console.log("---------------------------------------------");
        console.log("---------------------------------------------");
        console.log("USER_A share balance: ", shareUserA);
        console.log("---------------------------------------------");
        console.log("USER_B share balance: ", shareUserB);

    }
}

Impact

The severity of this issue is high. Every share purchase under the current mechanism can result in financial losses for users, with excess funds becoming permanently locked in the contract.

Code Snippet

The problematic code can be found in GSPFunding.sol:

• GSPFunding.sol#L67-L71

Tool used

• Manual Review
• Foundry

Recommendation

To rectify this issue, it is recommended that the share calculation in buyShares() should accurately reflect the amounts of both base token and token quotes. This change will ensure fair share distribution and prevent the permanent locking of user funds.

Duplicate of #74