stuart_the_minion - mintUBIFromInterest(), mintUBIFromReserveBalance() and mintRewardFromReserveRatio() will be denied when the exchange reserve balance is 0

[sherlock-admin4]

stuart_the_minion

Medium

mintUBIFromInterest(), mintUBIFromReserveBalance() and mintRewardFromReserveRatio() will be denied when the exchange reserve balance is 0

Summary

When an exchange is created, the reserve balance can be 0 or any positive value.

By the way, the GoodDollarExchangeProvider::mintFromInterest() will be reverted due to division by zero when the reserve balance is 0, denying mintUBIFromInterest(), mintUBIFromReserveBalance() and mintRewardFromReserveRatio() in GoodDollarExpansionController.

Root Cause

The GoodDollarExpansionProvider::mintFromInterest() doesn't check if exchange.reserveBalance is zero.

  function mintFromInterest(
    bytes32 exchangeId,
    uint256 reserveInterest
  ) external onlyExpansionController whenNotPaused returns (uint256 amountToMint) {
    PoolExchange memory exchange = getPoolExchange(exchangeId);

    uint256 reserveinterestScaled = reserveInterest * tokenPrecisionMultipliers[exchange.reserveAsset];
    uint256 amountToMintScaled = unwrap(
@>    wrap(reserveinterestScaled).mul(wrap(exchange.tokenSupply)).div(wrap(exchange.reserveBalance))
    );
    amountToMint = amountToMintScaled / tokenPrecisionMultipliers[exchange.tokenAddress];

    exchanges[exchangeId].tokenSupply += amountToMintScaled;
    exchanges[exchangeId].reserveBalance += reserveinterestScaled;

    return amountToMint;
  }

Therefore, when the reserve balance is 0, this function will be reverted with the division error.

Internal pre-conditions

No response

External pre-conditions

No response

Attack Path

No response

Impact

Minting UBI from interest and reserve balance will be denied.

I think, creating an exchange with 0 reserve balance doesn't require a malicious owner, and there exists possibility that an owner can make a mistake because there's no restriction on input reserve balance.

PoC

POC Test:

// SPDX-License-Identifier: GPL-3.0-or-later
pragma solidity 0.8.18;
// solhint-disable func-name-mixedcase, var-name-mixedcase, state-visibility
// solhint-disable const-name-snakecase, max-states-count, contract-name-camelcase

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

import { Test } from "forge-std/Test.sol";
import { ERC20 } from "openzeppelin-contracts-next/contracts/token/ERC20/ERC20.sol";

import { ERC20Mock } from "openzeppelin-contracts-next/contracts/mocks/ERC20Mock.sol";
import { GoodDollarExchangeProvider } from "contracts/goodDollar/GoodDollarExchangeProvider.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";
import { IReserve } from "contracts/interfaces/IReserve.sol";

import { GoodDollarExpansionControllerHarness } from "test/utils/harnesses/GoodDollarExpansionControllerHarness.sol";

import { GoodDollarExpansionControllerTest } from "../goodDollar/GoodDollarExpansionController.t.sol";

contract ExpansionControllerZeroReserveBalanceTest is GoodDollarExpansionControllerTest {
  GoodDollarExpansionController expansionController;
  address brokerAddress;

  IBancorExchangeProvider.PoolExchange public poolExchange3;
  ERC20 public reserveTokenNew;

  function setUp() public override {
    super.setUp();

    reserveTokenNew = new ERC20("USDC", "USDC");
    poolExchange3 = IBancorExchangeProvider.PoolExchange({
      reserveAsset: address(reserveTokenNew),
      tokenAddress: address(token),
      tokenSupply: 0 * 1e18,
      reserveBalance: 0 * 1e18,
      reserveRatio: 1e8 * 0.28571428,
      exitContribution: 1e8 * 0.1
    });

    brokerAddress = makeAddr("Broker");
    
    vm.mockCall(
      reserveAddress,
      abi.encodeWithSelector(IReserve(reserveAddress).isStableAsset.selector, address(token)),
      abi.encode(true)
    );
    vm.mockCall(
      reserveAddress,
      abi.encodeWithSelector(IReserve(reserveAddress).isCollateralAsset.selector, address(reserveTokenNew)),
      abi.encode(true)
    );

    expansionController = initializeGoodDollarExpansionController();

    exchangeProvider = address(new GoodDollarExchangeProvider(false));
    IGoodDollarExchangeProvider(exchangeProvider).initialize(brokerAddress, reserveAddress, address(expansionController), avatarAddress);

    expansionController.setGoodDollarExchangeProvider(exchangeProvider);

    vm.startPrank(avatarAddress);
    exchangeId = IBancorExchangeProvider(exchangeProvider).createExchange(poolExchange3);
    expansionController.setExpansionConfig(exchangeId, expansionRate, expansionFrequency);
    vm.stopPrank();
  }

  function test_mintUBI_zeroBalance() public {
    uint256 additionalReserveBalance = 1000e18;

    deal(address(reserveTokenNew), reserveAddress, pool.reserveBalance + additionalReserveBalance);

    // @audit Minting UBI from reserve balance expects to be reverted
    vm.expectRevert(stdError.divisionError);
    uint256 amountMinted = expansionController.mintUBIFromReserveBalance(exchangeId);

    // @audit Minting UBI from interest expects to be reverted
    vm.expectRevert(stdError.divisionError);
    vm.prank(brokerAddress);
    expansionController.mintUBIFromInterest(exchangeId, additionalReserveBalance);

    // @audit Minting rewards expects to be reverted
    vm.expectRevert(stdError.divisionError);
    vm.prank(avatarAddress);
    expansionController.mintRewardFromReserveRatio(exchangeId, address(0x0111), 1e18);
  }
}

Output:

[PASS] test_mintUBI_zeroBalance() (gas: 335787)
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 3.28ms (617.00µs CPU time)

As can be seen from the logs, three minting functions are denied due to zero reserve balance.

Mitigation

I'd suggest adding the minimum limit of reserve balance when creating an exchange.

[sherlock-admin2]

The protocol team fixed this issue in the following PRs/commits:
mento-protocol/mento-core#558