Ondo Finance - 0xAkira's results

Lines of code

https://github.com/code-423n4/2024-03-ondo-finance/blob/main/contracts/ousg/ousgInstantManager.sol#L554-L560 https://github.com/code-423n4/2024-03-ondo-finance/blob/main/contracts/ousg/ousgInstantManager.sol#L567-L573

Vulnerability details

Impact

In functions setMintFee() and setRedeemFee() admin sets new commission for minting and redeeming respectively. The first check is that mintFee/redeemFee (old commission) should be less than 200, if this condition is not met, the corresponding error is called.

function setMintFee(
    uint256 _mintFee
  ) external override onlyRole(CONFIGURER_ROLE) {
    require(mintFee < 200, "OUSGInstantManager::setMintFee: Fee too high");
    emit MintFeeSet(mintFee, _mintFee);
    mintFee = _mintFee;
  }

  function setRedeemFee(
    uint256 _redeemFee
  ) external override onlyRole(CONFIGURER_ROLE) {
    require(redeemFee < 200, "OUSGInstantManager::setRedeemFee: Fee too high");
    emit RedeemFeeSet(redeemFee, _redeemFee);
    redeemFee = _redeemFee;
  }

But in these functions there is no check on the input data uint256 _mintFee/_redeemFee (new commission). If the admin sets _mintFee/_redeemFee >=200, he will not be able to use these functions and set new commissions in the future, because the condition will not be met.

The lack of the ability to set new commissions violates the functionality of the contract.

Proof of Concept

1. PoC for setMintFee()

 function testSetMintFee() public {
    vm.startPrank(OUSG_GUARDIAN);

    ousgInstantManager.setMintFee(300);

    ousgInstantManager.setMintFee(100);
  }

A message is returned that the fee is too high

[FAIL. Reason: revert: OUSGInstantManager::setMintFee: Fee too high] testSetMintFee() (gas: 35732)

2. PoC for setRedeemFee()

function testSetRedeemFee() public {
    vm.startPrank(OUSG_GUARDIAN);

    ousgInstantManager.setRedeemFee(300);

    ousgInstantManager.setRedeemFee(100);
  }

A message is returned that the fee is too high

[FAIL. Reason: revert: OUSGInstantManager::setRedeemFee: Fee too high] testSetRedeemFee() (gas: 35746)

Tools Used

forge

Recommended Mitigation Steps

It is worth considering adding another check on the input data so that it is also less than 200

function setMintFee(
    uint256 _mintFee
  ) external override onlyRole(CONFIGURER_ROLE) {
+     require(mintFee < 200 && _mintFee < 200, "OUSGInstantManager::setMintFee: Fee too high");
    
    emit MintFeeSet(mintFee, _mintFee);
    mintFee = _mintFee;
  }

  function setRedeemFee(
    uint256 _redeemFee
  ) external override onlyRole(CONFIGURER_ROLE) {
+     require(redeemFee < 200 && _redeemFee < 200, "OUSGInstantManager::setRedeemFee: Fee too high");
    emit RedeemFeeSet(redeemFee, _redeemFee);
    redeemFee = _redeemFee;
  }

or simply replace mintFee/redeemFee with _mintFee/_redeemFee in the require()

function setMintFee(
    uint256 _mintFee
  ) external override onlyRole(CONFIGURER_ROLE) {
+     require(_mintFee < 200, "OUSGInstantManager::setMintFee: Fee too high");
    emit MintFeeSet(mintFee, _mintFee);
    mintFee = _mintFee;
  }

  function setRedeemFee(
    uint256 _redeemFee
  ) external override onlyRole(CONFIGURER_ROLE) {
+     require(_redeemFee < 200, "OUSGInstantManager::setRedeemFee: Fee too high");
    emit RedeemFeeSet(redeemFee, _redeemFee);
    redeemFee = _redeemFee;
  }

Assessed type

DoS