zarkk01 - GoodDollarExpansionController::mintUBIFromReserveBalance() will always revert or misbehave for reserveAssets with less than 18 decimals.

[sherlock-admin3]

zarkk01

High

GoodDollarExpansionController::mintUBIFromReserveBalance() will always revert or misbehave for reserveAssets with less than 18 decimals.

Summary

GoodDollarExpansionController::mintUBIFromReserveBalance() does not normalise the contractReserveBalance to 18 decimals making the function unusable for reserveAssets with less than 18 decimals since it will always revert or misbehave.

Root Cause

In GoodDollarExpansionController::mintUBIFromReserveBalance(), there is no normalisation to 18 decimals for the contractReserveBalance which will be in the native decimals of the ERC20. However, the reserveBalance of the PoolExchange is normalised to 18 decimals. The subtraction of them will, most likely, result to the transaction to be reverted since it will try to do e6 - e18. As we can see in the implementation:

  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);
    }
  }

Link to code

As we can see the subtraction tries to subtract a normalised amount(exchange.reserveBalance) from a non normalised amount in native decimals ( contractReserveBalance). It is, almost, sure that it will revert or misbehave.

Internal pre-conditions

1. The reserveAsset of the PoolExchange to have less than 18 decimals, which is very likely and that's way Mento has introduced the tokenPrecisionMultipliers mapping on BancorExchangeProvider.

External pre-conditions

1. Someone to call GoodDollarExpansionController::mintUBIFromReserveBalance(), in order to mint UBI (G$ tokens) from the additionalReserveBalance of reserve contract.

Attack Path

1.PoolExchange with a reserveAsset with less than 18 decimals is created (6 decimals for example). This is expected from Mento and that's why the tokenPrecisionMultipliers have been introduced in GoodDollarExchangeProvider.
2. GoodDollarExpansionController::mintUBIFromReserveBalance() function get called so to mint some G$ from the reserve balance.
3. The transaction will revert and the functionality is unusable for ever.

Impact

The impact of this bug is that mintUBIFromReserveBalance() functionality will be broken for exchanges with reserveAsset with less than 18 decimals, and there will be absolutely no way to mint UBI from the excess balance of Reserve contract. This is one of the main functionalities of the GoodDollarExpansionController and it will be broken for all assets which don't have 18 decimals.

PoC

No response

Mitigation

To mitigate successfully this vulnerability, normalise the contractReserveBalance to 18 decimals before the subtraction with exchange.reserveBalance .