0xShitgem - Minting UBI from reserve balance is incorrectly handled

[sherlock-admin4]

0xShitgem

Medium

Minting UBI from reserve balance is incorrectly handled

Summary

In every function/transaction where we deal with tokens, they get scaled to 18 decimals and saved to struct. If reserveToken happens to be token with decimals less than 18 decimals - function mintUBIFromReserveBalance won't correctly perform calculation or even revert.

  function mintUBIFromReserveBalance(bytes32 exchangeId) external returns (uint256 amountMinted) {
    IBancorExchangeProvider.PoolExchange memory exchange = IBancorExchangeProvider(address(goodDollarExchangeProvider))
      .getPoolExchange(exchangeId);

    uint256 contractReserveBalance = IERC20(exchange.reserveAsset).balanceOf(reserve);
@>  uint256 additionalReserveBalance = contractReserveBalance - exchange.reserveBalance;
    if (additionalReserveBalance > 0) {
      amountMinted = goodDollarExchangeProvider.mintFromInterest(exchangeId, additionalReserveBalance);
      IGoodDollar(exchange.tokenAddress).mint(address(distributionHelper), amountMinted);

      // Ignored, because contracts only interacts with trusted contracts and tokens
      // slither-disable-next-line reentrancy-events
      emit InterestUBIMinted(exchangeId, amountMinted);
    }
  }

As in above code, IERC20(exchange.reserveAsset).balanceOf(reserve) will return token with their correct decimals (for example 6), but exchange.reserveBalance will return scaled tokens which will lead to underflow/overflow error.

Root Cause

The root cause is handling incorrectly decimals inside mintUBIFromReserveBalance.

Internal pre-conditions

Tokens with less than 18 decimals

External pre-conditions

Tokens with less than 18 decimals

Attack Path

No response

Impact

Function mintUBIFromReserveBalance is unable to execute correctly.

PoC

Add below code to mento-core/test/unit/goodDollar

// SPDX-License-Identifier: GPL-3.0-or-later 
pragma solidity 0.8.18;

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

import { ERC20Mock } from "openzeppelin-contracts-next/contracts/mocks/ERC20Mock.sol";
import { IGoodDollarExchangeProvider } from "contracts/interfaces/IGoodDollarExchangeProvider.sol";
import { GoodDollarExpansionController } from "contracts/goodDollar/GoodDollarExpansionController.sol";

import { IGoodDollarExpansionController } from "contracts/interfaces/IGoodDollarExpansionController.sol";
import { IGoodDollarExchangeProvider } from "contracts/interfaces/IGoodDollarExchangeProvider.sol";
import { IBancorExchangeProvider } from "contracts/interfaces/IBancorExchangeProvider.sol";
import { IDistributionHelper } from "contracts/goodDollar/interfaces/IGoodProtocol.sol";

contract GoodDollarExpansionController6DecTest is Test {
  ERC20Mock public reserveToken;   
  ERC20Mock public token;

  GoodDollarExpansionController expansionController;

  address public exchangeProvider;
  address public distributionHelper;
  address public reserveAddress;
  address public avatarAddress;

  bytes32 exchangeId = "ExchangeId";

  uint64 expansionRate = 1e18 * 0.01;
  uint32 expansionFrequency = uint32(1 days);

  IBancorExchangeProvider.PoolExchange pool;

  function setUp() public {
    reserveToken = new ERC20Mock("cUSD", "cUSD", address(this), 1);
    token = new ERC20Mock("Good$", "G$", address(this), 1);

    exchangeProvider = makeAddr("ExchangeProvider");
    distributionHelper = makeAddr("DistributionHelper");
    reserveAddress = makeAddr("Reserve");
    avatarAddress = makeAddr("Avatar");

    pool = IBancorExchangeProvider.PoolExchange({
      reserveAsset: address(reserveToken),
      tokenAddress: address(token),
      tokenSupply: 7 * 1e9 * 1e18,
      reserveBalance: 200_000 * 1e18,
      reserveRatio: 0.2 * 1e8, // 20%
      exitContribution: 0.1 * 1e8 // 10%
    });

    vm.mockCall(
      exchangeProvider,
      abi.encodeWithSelector(IBancorExchangeProvider(exchangeProvider).getPoolExchange.selector),
      abi.encode(pool)
    );

    expansionController = initializeGoodDollarExpansionController(); 
    
    vm.prank(avatarAddress);
    expansionController.setExpansionConfig(exchangeId, expansionRate, expansionFrequency);

    vm.mockCall(
      exchangeProvider,
      abi.encodeWithSelector(IGoodDollarExchangeProvider(exchangeProvider).mintFromExpansion.selector),
      abi.encode(1000e18)
    );

    vm.mockCall(
      distributionHelper,
      abi.encodeWithSelector(IDistributionHelper(distributionHelper).onDistribution.selector),
      abi.encode(true)
    );
  }

  function initializeGoodDollarExpansionController() internal returns (GoodDollarExpansionController) {
    GoodDollarExpansionController expansionController = new GoodDollarExpansionController(false);

    expansionController.initialize(exchangeProvider, distributionHelper, reserveAddress, avatarAddress);

    return expansionController;
  }

  function test_mintUBIFromReserveBalance_6Dec() public {
    uint256 amountToMint = 1000e18;
    uint256 additionalReserveBalance = 10000e6;

    // inside struct we have 18 decimals however our reserve is 6 dec, that's why division
    uint256 actualReserveBalance = pool.reserveBalance / 1e12;
    assertEq(actualReserveBalance, 200_000e6);

    deal(address(reserveToken), reserveAddress, actualReserveBalance + additionalReserveBalance);

    uint256 distributionHelperBalanceBefore = token.balanceOf(distributionHelper);

    uint256 amountMinted = expansionController.mintUBIFromReserveBalance(exchangeId);
    // revert with underflow/overflow error

  }
}

Mitigation

Scale balanceOf and later divide by precision.