0xpetern - Access Control Vulnerability in mintFromInterest Allows Unauthorized Token Minting

[sherlock-admin2]

0xpetern

Medium

Access Control Vulnerability in mintFromInterest Allows Unauthorized Token Minting

Summary

The mintUBIFromInterest in GoodDollarExpansion.sol function does not have an access control mechanism itself.

function mintUBIFromInterest(bytes32 exchangeId, uint256 reserveInterest) external {
    require(reserveInterest > 0, "Reserve interest must be greater than 0");
    IBancorExchangeProvider.PoolExchange memory exchange = IBancorExchangeProvider(address(goodDollarExchangeProvider))
      .getPoolExchange(exchangeId);

    uint256 amountToMint = goodDollarExchangeProvider.mintFromInterest(exchangeId, reserveInterest);

    require(IERC20(exchange.reserveAsset).transferFrom(msg.sender, reserve, reserveInterest), "Transfer failed");
    IGoodDollar(exchange.tokenAddress).mint(address(distributionHelper), amountToMint);

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

This means anyone can call it, and they can potentially trigger the minting of tokens.
Then, when mintUBIFromInterest calls mintFromInterest, the msg.sender is the goodDollarExpansion.sol contract. Since msg.sender in mintFromInterest is the contract address (goodDollarExpansion.sol) , it passes the onlyExpansionController check.

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

Here's the function that sets the expansion controller address.

function setExpansionController(address _expansionController) public onlyOwner {
require(_expansionController != address(0), "ExpansionController address must be set");
 expansionController = IGoodDollarExpansionController(_expansionController);
    emit ExpansionControllerUpdated(_expansionController);
  }

So when a user, whether malicious or not interacts with the mintUBIFromInterest and makes an external call to mintFromInterest where the msg.sender is expansionController , the check will pass. Because

modifier onlyExpansionController() {
require(msg.sender == address(expansionController), "Only ExpansionController can call this function");
_;
}

An attacker could mint an arbitrary number of tokens without proper authorization, resulting in inflation, dilution of existing token holders’ assets, and potential collapse of the token's value.

Root Cause

https://github.com/sherlock-audit/2024-10-mento-update/blob/main/mento-core/contracts/goodDollar/GoodDollarExpansionController.sol#L137C3-L150C4

https://github.com/sherlock-audit/2024-10-mento-update/blob/main/mento-core/contracts/goodDollar/GoodDollarExchangeProvider.sol#L172C3-L188C4.

In GoodDollarExpansion.sol:147, an unauthorized user or attacker can bypass the onlyExpansionController access control mechanism by exploiting external function calls where the msg.sender is an expansion controller.

Internal pre-conditions

1. The contract must be designed such that mintUBIFromInterest that doesn't have access control and callable by anyone calls the external function mintFromInterest.
2. The goodDollarExchangeProvider must have the onlyExpansionController modifier that allows its address to pass the access control check because the call came from an expansion controller contract.

External pre-conditions

1. A malicious user must be able to interact with the contract and call the mintUBIFromInterest function.
2. There must be no additional check to detect that the original caller is an attacker.

Attack Path

1. A user (malicious or not) can invoke mintUBIFromInterest.
2. This function calls mintFromInterest on goodDollarExchangeProvider.
3. Because msg.sender in mintFromInterest is the GoodDollarExpansion contract, the onlyExpansionController check passes.
   The attacker could effectively mint tokens without proper authorization.

Impact

1. Financial Loss: An attacker could mint an arbitrary number of tokens without proper authorization, resulting in inflation, dilution of existing token holders’ assets, and potential collapse of the token's value.
2. Havoc: If an attacker gains unauthorized access, they can destabilize the entire system by minting excess tokens, undermining trust in the token economy and damaging the project’s reputation.

PoC

1. Invoke the Function: An attacker creates a transaction that calls mintUBIFromInterest with valid parameters.
2. Trigger External Call: The transaction executes, calling mintFromInterest through the goodDollarExpansion contract.
3. Mint Tokens: The attacker successfully mints tokens due to the absence of proper access controls.

Mitigation

Consider this:

1. Use a mapping to store addresses associated with each role. For example:

mapping(address => bool) public expansionControllers;

2. Then, create modifiers to restrict access. For example

   modifier onlyExpansionController() {
          require(expansionControllers[msg.sender], "Not an Expansion Controller");
          _;
      }

4. Provide functions to assign and revoke roles. For example:

 function assignExpansionController(address _controller) public onlyOwner {
          expansionControllers[_controller] = true;
      }
      
function revokeExpansionController(address _controller) public onlyOwner {
          expansionControllers[_controller] = false;
      }

5. Finally, Use the role modifiers in critical functions to enforce access control.