`MintExchangeRate` can change by admin any time, `claimMint()` allow claim for anyone can lead to a front-run attack when a wrong `MintExchangeRate` been set

[code423n4]

Lines of code

https://github.com/code-423n4/2023-01-ondo/blob/f3426e5b6b4561e09460b2e6471eb694efdd6c70/contracts/cash/CashManager.sol#L241-L269

Vulnerability details

Impact

The mint exchange rate of the epoch is set by the admin. The current system takes into account that if the admin sets an improper value, it can be corrected via the overrideExchangeRate() function.

If the previous incorrect value was lower, but not low enough to trigger an automatic pause, and then the admin sets a correct higher mint exchange rate, an attacker who observes this transaction could front-run and claimMint() for other users, except himself. forcing others to accept the incorrect, lower exchange rate.

Proof of Concept

https://github.com/code-423n4/2023-01-ondo/blob/f3426e5b6b4561e09460b2e6471eb694efdd6c70/contracts/cash/CashManager.sol#L241-L269

function claimMint(
    address user,
    uint256 epochToClaim
  ) external override updateEpoch nonReentrant whenNotPaused checkKYC(user) {
    uint256 collateralDeposited = mintRequestsPerEpoch[epochToClaim][user];
    if (collateralDeposited == 0) {
      revert NoCashToClaim();
    }
    if (epochToExchangeRate[epochToClaim] == 0) {
      revert ExchangeRateNotSet();
    }

    // Get the amount of CASH due at a given rate per epoch
    uint256 cashOwed = _getMintAmountForEpoch(
      collateralDeposited,
      epochToClaim
    );

    mintRequestsPerEpoch[epochToClaim][user] = 0;
    cash.mint(user, cashOwed);

    emit MintCompleted(
      user,
      cashOwed,
      collateralDeposited,
      epochToExchangeRate[epochToClaim],
      epochToClaim
    );
  }

https://github.com/code-423n4/2023-01-ondo/blob/f3426e5b6b4561e09460b2e6471eb694efdd6c70/contracts/cash/CashManager.sol#L487-L493

 function _getMintAmountForEpoch(
    uint256 collateralAmountIn,
    uint256 epoch
  ) private view returns (uint256 cashAmountOut) {
    uint256 amountE24 = _scaleUp(collateralAmountIn) * 1e6;
    cashAmountOut = amountE24 / epochToExchangeRate[epoch];
  }

https://github.com/code-423n4/2023-01-ondo/blob/f3426e5b6b4561e09460b2e6471eb694efdd6c70/contracts/cash/CashManager.sol#L366-L385

function overrideExchangeRate(
    uint256 correctExchangeRate,
    uint256 epochToSet,
    uint256 _lastSetMintExchangeRate
  ) external override updateEpoch onlyRole(MANAGER_ADMIN) {
    if (epochToSet >= currentEpoch) {
      revert MustServicePastEpoch();
    }
    uint256 incorrectRate = epochToExchangeRate[epochToSet];
    epochToExchangeRate[epochToSet] = correctExchangeRate;
    if (_lastSetMintExchangeRate != 0) {
      lastSetMintExchangeRate = _lastSetMintExchangeRate;
    }
    emit MintExchangeRateOverridden(
      epochToSet,
      incorrectRate,
      correctExchangeRate,
      lastSetMintExchangeRate
    );
  }

1. Admin call setMintExchangeRate() set exchange rate for epoch 1 to 100

2. Admin discovered that 100 is the incorrect exchange rate for epoch 1, the correct value should be set to 110, so the admin call overrideExchangeRate() to set the exchange rate to 110.

3. The attacker observes the admin's overrideExchangeRate() transaction and quickly initiates transactions call claimMint() front-run admin's tx, claim the cash for other users in epoch 1 let them accept the exchange rate: 100.

Recommended Mitigation Steps

Consider only allow msg.sender claimMint for himself

[c4-judge]

Duplicate of https://github.com/code-423n4/2023-01-ondo-findings/issues/260

[c4-judge]

trust1995 marked the issue as grade-b