Ondo Finance - radev_sw's results

Institutional-Grade Finance, Now Onchain.

General Information

Platform: Code4rena

Start Date: 29/03/2024

Pot Size: $36,500 USDC

Total HM: 5

Participants: 72

Period: 5 days

Judge: 3docSec

Total Solo HM: 1

Id: 357

League: ETH

Ondo Finance

Findings Distribution

Researcher Performance

Rank: 9/72

Findings: 3

Award: $563.06

🌟 Selected for report: 0

🚀 Solo Findings: 0

Findings Information

🌟 Selected for report: carrotsmuggler

Also found by: 0xCiphky, 0xmystery, Breeje, dvrkzy, radev_sw

Labels

bug
2 (Med Risk)
insufficient quality report
satisfactory
edited-by-warden
:robot:_124_group
duplicate-142

Awards

490.6256 USDC - $490.63

External Links

Lines of code

https://github.com/code-423n4/2024-03-ondo-finance/blob/main/contracts/ousg/ousgInstantManager.sol#L335-L470

Vulnerability details

Impact

The OUSGInstantManager.sol contract governs the minting and redemption processes for OUSG/rOUSG tokens within the Ondo Finance ecosystem. The contract employs minimumDepositAmount and minimumRedemptionAmount as mechanisms to set thresholds for the least amount of USDC that can be used to mint OUSG tokens and the minimum amount of OUSG tokens that can be redeemed back into USDC, respectively. These parameters are crucial for maintaining operational integrity and preventing spam transactions.

  // Minimum amount of USDC that must be deposited to mint OUSG or rOUSG
  // Denoted in 6 decimals for USDC
  uint256 public minimumDepositAmount = 100_000e6;

  // Minimum amount of USDC that must be redeemed for to redeem OUSG or rOUSG
  // Denoted in 6 decimals for USDC
  uint256 public minimumRedemptionAmount = 50_000e6;

However, they can also restrict user actions based on changing protocol conditions. For example, a user who mints OUSG tokens under one set of conditions finds themselves can be unable to redeem their tokens when the minimumRedemptionAmount (or both minimumDepositAmount and minimumRedemptionAmount) is later increased (new minimumRedemptionAmount becomes greater than the "old" minimumDepositAmount). This change can trap the user's tokens within the protocol under the new conditions, despite their actions being compliant with the terms at the time of minting.

This also leads to broken protocol invariant: USDC is not permanently locked.

Proof of Concept

  1. Alice mints OUSG tokens when the minimumDepositAmount is 60,000 USDC, and the minimumRedemptionAmount is 50,000 OUSG.
  2. The protocol later increases these thresholds to 80,000 USDC and 70,000 OUSG, respectively, to adjust to new economic conditions.
  3. Alice, wanting to redeem her tokens, discovers she is unable to do so because her holdings do not meet the new minimum redemption threshold.
function _mint(
  uint256 usdcAmountIn,
  address to
) internal returns (uint256 ousgAmountOut) {
  require(
    IERC20Metadata(address(usdc)).decimals() == 6,
    "OUSGInstantManager::_mint: USDC decimals must be 6"
  );
  require(
    usdcAmountIn >= minimumDepositAmount,
    "OUSGInstantManager::_mint: Deposit amount too small"
  );
  _checkAndUpdateInstantMintLimit(usdcAmountIn);
  if (address(investorBasedRateLimiter) != address(0)) {
    investorBasedRateLimiter.checkAndUpdateMintLimit(msg.sender, usdcAmountIn);
  }

  require(
    usdc.allowance(msg.sender, address(this)) >= usdcAmountIn,
    "OUSGInstantManager::_mint: Allowance must be given to OUSGInstantManager"
  );

  uint256 usdcfees = _getInstantMintFees(usdcAmountIn);
  uint256 usdcAmountAfterFee = usdcAmountIn - usdcfees;

  // Calculate the mint amount based on mint fees and usdc quantity
  uint256 ousgPrice = getOUSGPrice();
  ousgAmountOut = _getMintAmount(usdcAmountAfterFee, ousgPrice);

  require(
    ousgAmountOut > 0,
    "OUSGInstantManager::_mint: net mint amount can't be zero"
  );

  // Transfer USDC
  if (usdcfees > 0) {
    usdc.transferFrom(msg.sender, feeReceiver, usdcfees);
  }
  usdc.transferFrom(msg.sender, usdcReceiver, usdcAmountAfterFee);

  emit MintFeesDeducted(msg.sender, feeReceiver, usdcfees, usdcAmountIn);

  ousg.mint(to, ousgAmountOut);
}
function _redeem(
  uint256 ousgAmountIn
) internal returns (uint256 usdcAmountOut) {
  require(
    IERC20Metadata(address(usdc)).decimals() == 6,
    "OUSGInstantManager::_redeem: USDC decimals must be 6"
  );
  require(
    IERC20Metadata(address(buidl)).decimals() == 6,
    "OUSGInstantManager::_redeem: BUIDL decimals must be 6"
  );
  uint256 ousgPrice = getOUSGPrice();
  uint256 usdcAmountToRedeem = _getRedemptionAmount(ousgAmountIn, ousgPrice);

  require(
    usdcAmountToRedeem >= minimumRedemptionAmount,
    "OUSGInstantManager::_redeem: Redemption amount too small"
  );
  _checkAndUpdateInstantRedemptionLimit(usdcAmountToRedeem);

  if (address(investorBasedRateLimiter) != address(0)) {
    investorBasedRateLimiter.checkAndUpdateRedeemLimit(
      msg.sender,
      usdcAmountToRedeem
    );
  }

  uint256 usdcFees = _getInstantRedemptionFees(usdcAmountToRedeem);
  usdcAmountOut = usdcAmountToRedeem - usdcFees;
  require(
    usdcAmountOut > 0,
    "OUSGInstantManager::_redeem: redeem amount can't be zero"
  );

  ousg.burn(ousgAmountIn);

  uint256 usdcBalance = usdc.balanceOf(address(this));

  if (usdcAmountToRedeem >= minBUIDLRedeemAmount) {
    // amount of USDC needed is over minBUIDLRedeemAmount, do a BUIDL redemption
    // to cover the full amount
    _redeemBUIDL(usdcAmountToRedeem);
  } else if (usdcAmountToRedeem > usdcBalance) {
    // There isn't enough USDC held by this contract to cover the redemption,
    // so we perform a BUIDL redemption of BUIDL's minimum required amount.
    // The remaining amount of USDC will be held in the contract for future redemptions.
    _redeemBUIDL(minBUIDLRedeemAmount);
    emit MinimumBUIDLRedemption(
      msg.sender,
      minBUIDLRedeemAmount,
      usdcBalance + minBUIDLRedeemAmount - usdcAmountToRedeem
    );
  } else {
    // We have enough USDC sitting in this contract already, so use it
    // to cover the redemption and fees without redeeming more BUIDL.
    emit BUIDLRedemptionSkipped(
      msg.sender,
      usdcAmountToRedeem,
      usdcBalance - usdcAmountToRedeem
    );
  }

  if (usdcFees > 0) {
    usdc.transfer(feeReceiver, usdcFees);
  }
  emit RedeemFeesDeducted(msg.sender, feeReceiver, usdcFees, usdcAmountOut);

  usdc.transfer(msg.sender, usdcAmountOut);
}

Tools Used

Manual Review

I would suggest two things:

  1. Consider implementing rules that grandfather existing transactions under the conditions active at the time of their initiation. This could ensure that changes in parameters do not retroactively impact users' ability to redeem tokens.

  2. Change the implementation of as follow:

  function setMinimumRedemptionAmount(
    uint256 _minimumRedemptionAmount
  ) external override onlyRole(CONFIGURER_ROLE) {
    require(
      _minimumRedemptionAmount >= FEE_GRANULARITY,
      "setMinimumRedemptionAmount: Amount too small"
    );
+   require(
+     _minimumRedemptionAmount < minimumDepositAmount,
+     "setMinimumRedemptionAmount: Minimum Redemption Amount cannot be greater than Minimum Deposit Amount"
+   );
    emit MinimumRedemptionAmountSet(
      minimumRedemptionAmount,
      _minimumRedemptionAmount
    );
    minimumRedemptionAmount = _minimumRedemptionAmount;
  }

Assessed type

Context

#0 - 0xRobocop

2024-04-04T01:48:31Z

Consider QA. I do not see a security concern. The impact described is overinflated:

This also leads to broken protocol invariant: USDC is not permanently locked.

#1 - c4-pre-sort

2024-04-04T01:48:53Z

0xRobocop marked the issue as insufficient quality report

#2 - 3docSec

2024-04-09T12:34:07Z

I agree "This also leads to broken protocol invariant: USDC is not permanently locked." is an overstatement. Apart from this, the finding fits well under the #142 umbrella, especially with #44.

#3 - c4-judge

2024-04-09T12:34:25Z

3docSec marked the issue as duplicate of #142

#4 - c4-judge

2024-04-09T12:34:29Z

3docSec marked the issue as satisfactory

Findings Information

Awards

64.1515 USDC - $64.15

Labels

bug
2 (Med Risk)
satisfactory
edited-by-warden
:robot:_26_group
duplicate-32

External Links

Lines of code

https://github.com/code-423n4/2024-03-ondo-finance/blob/main/contracts/ousg/rOUSG.sol#L624-L640

Vulnerability details

Impact

The rOUSG.sol contract is part of Ondo Finance's ecosystem. A critical aspect of rOUSG involves compliance with Know Your Customer (KYC) regulations (aka sanctions lists), which are enforced through checks in the _beforeTokenTransfer() function to ensure only verified users can transact. The KYCRegistry acts as a whitelist for Ondo's permissioned tokens. OUSG and rOUSG token contracts query this registry to check that addresses are KYC verified before executing certain actions. Users are added and removed from the KYC list via permissioned functions. However, a notable flaw in the contract's design is the lack of an exception for administrative roles to bypass these checks for the purpose of burning tokens belonging to users who have become unverified or sanctioned after initially passing KYC verification. This leads to a situation, where users who lose their KYC status or get sanctioned cannot move their tokens, effectively locking their funds within the protocol (sanctioned user's funds are locked). Another flaw is that even administrators or designated burners with DEFAULT_ADMIN_ROLE/BURNER_ROLE cannot redeem or burn these locked tokens, preventing the protocol from managing assets or complying with potential regulatory orders to seize assets.

Proof of Concept

  • Burn Flow:
/**
 * @notice Admin burn function to burn rOUSG tokens from any account
 * @param _account The account to burn tokens from
 * @param _amount  The amount of rOUSG tokens to burn
 * @dev Transfers burned shares (OUSG) to `msg.sender`
 */
function burn(
  address _account,
  uint256 _amount
) external onlyRole(BURNER_ROLE) {
  uint256 ousgSharesAmount = getSharesByROUSG(_amount);
  if (ousgSharesAmount < OUSG_TO_ROUSG_SHARES_MULTIPLIER)
    revert UnwrapTooSmall();

  _burnShares(_account, ousgSharesAmount);

  ousg.transfer(msg.sender, ousgSharesAmount / OUSG_TO_ROUSG_SHARES_MULTIPLIER);
  emit Transfer(address(0), msg.sender, getROUSGByShares(ousgSharesAmount));
  emit TransferShares(_account, address(0), ousgSharesAmount);
}

function _burnShares(
  address _account,
  uint256 _sharesAmount
) internal whenNotPaused returns (uint256) {
  require(_account != address(0), "BURN_FROM_THE_ZERO_ADDRESS");

  _beforeTokenTransfer(_account, address(0), _sharesAmount);

  uint256 accountShares = shares[_account];
  require(_sharesAmount <= accountShares, "BURN_AMOUNT_EXCEEDS_BALANCE");

  totalShares -= _sharesAmount;

  shares[_account] = accountShares - _sharesAmount;

  return totalShares;
}

function _beforeTokenTransfer(address from, address to, uint256) internal view {
  // Check constraints when `transferFrom` is called to facliitate
  // a transfer between two parties that are not `from` or `to`.
  if (from != msg.sender && to != msg.sender) {
    require(_getKYCStatus(msg.sender), "rOUSG: 'sender' address not KYC'd");
  }

  if (from != address(0)) {
    // If not minting
    require(_getKYCStatus(from), "rOUSG: 'from' address not KYC'd");
  }

  if (to != address(0)) {
    // If not burning
    require(_getKYCStatus(to), "rOUSG: 'to' address not KYC'd");
  }
}

It is understood that the users can not mint nor burn and instant mint and redeem via ousgInstantManager.sol contract, because the rOUSG.sol#_beforeTokenTransfer() function check if the users have KYCRegistry. And it is also understood that the protocol team knows about this. But I still believe the admin should be able to redeem those funds if users doesn't have KYCRegistry. And it is not possible for now because during redemption, the rOUSG.sol#_beforeTokenTransfer() function check, if the users have KYCRegistry. So as long as the user becomes unverified (get KYCRegistry /again/), the funds are completely locked and even the admin has no control over it. My main point is that the funds are permanently locked in the protocol. The issue relates to the fact that when users lose their KYC or are sanctioned, their funds are locked in (from their perspective). The fact those funds can't be claimed by delegated authorities is concerning.

Example Scenario:

  1. Alice, a verified user, acquires OUSG tokens.
  2. Alice later loses her KYC verification status due to regulatory or administrative reasons.
  3. The admin attempts to burn Alice's tokens to remove them from circulation or transfer them to a compliant wallet but finds that the operation is blocked due to the KYC check failing for Alice.

Tools Used

Manual Review

Assuming that the DEFAULT_ADMIN_ROLE/BURNER_ROLE can be trusted, I suggest removing KYC check during redemption if the msg.sender is DEFAULT_ADMIN_ROLE or BURNER_ROLE.

Assessed type

Context

#0 - c4-pre-sort

2024-04-04T05:11:38Z

0xRobocop marked the issue as duplicate of #237

#1 - c4-judge

2024-04-09T08:31:55Z

3docSec marked the issue as satisfactory

TitleSeverity
1Burning is centralizedLow
2Using an older Solidity compiler versionLow
3Outstanding USDC approvals during minting and OUSG approvals during redemption in OUSGInstantManager.sol contractLow
4usdcReceiver in OUSGInstantManager.sol contract cannot be changedLow
5The minBUIDLRedeemAmount state variable in OUSGInstantManager.sol can be changed, which is completely unwanted behaviour by contest READMELow
6Change the getOUSGPrice() function to check if price is greater or equals to MINIMUM_OUSG_PRICE valueLow
7No default fees configuration (mintFee and redeemFee)Low
8Missing calls to base initializers in rUSDYLow
9Consider bounding input array lengthLow
10Constant decimal valuesLow
11constructor/initialize function lacks parameter validationLow
12require() should be used instead of assert()Low
13Setters should prevent re-setting of the same valueLow
14Upgradeable contract is missing a __gap[50] storage variable at the end to allow for new storage variables in later versionsLow
15@openzeppelin/contracts should be upgraded to a newer versionLow
16Add inline comments for unnamed variablesNC
17Consider using SafeTransferLib.safeTransferETH() or Address.sendValue() for clearer semantic meaningNC
18Contract uses both require()/revert() as well as custom errorsNC
19Custom error without detailsNC
20Custom errors should be used rather than revert()/require()NC
21Event emit should emit a parameterNC
22Event is missing indexed fieldsNC
23Events are missing sender informationNC
24Events may be emitted out of order due to reentrancyNC
25Events should be emitted before external callsNC
26Events that mark critical parameter changes should contain both the old and the new valueNC
27Multiple NatSpec Bugs/ImprovementsNC
28Non-external/public state variables should begin with an underscoreNC
29Not using the latest versions of project dependenciesNC
30Prefer skip over revert model in iterationNC
31Use @inheritdoc for overridden functionsNC
32Use of override is unnecessaryNC
33Variables need not be initialized to zeroNC

1. Burning is centralized

GitHub Links

Description

The burning mechanism in the rOUSG contract is controlled by a designated BURNER_ROLE, which in turn is managed by the DEFAULT_ADMIN_ROLE. This setup centralizes the capability to burn tokens, granting the admin or any account with the BURNER_ROLE the power to remove tokens from circulation without the token holder's consent. In scenarios where the admin or burner accounts are either compromised or act maliciously, this could lead to unauthorized burning of user tokens, potentially leading to loss of funds for the token holders. Additionally, the control over burning could be exploited to manipulate the token's supply, impacting its overall economy and trust.

  /**
   * @notice Admin burn function to burn rOUSG tokens from any account
   * @param _account The account to burn tokens from
   * @param _amount  The amount of rOUSG tokens to burn
   * @dev Transfers burned shares (OUSG) to `msg.sender`
   */
  function burn(
    address _account,
    uint256 _amount
  ) external onlyRole(BURNER_ROLE) {
    uint256 ousgSharesAmount = getSharesByROUSG(_amount);
    if (ousgSharesAmount < OUSG_TO_ROUSG_SHARES_MULTIPLIER)
      revert UnwrapTooSmall();

    _burnShares(_account, ousgSharesAmount);

    ousg.transfer(
      msg.sender,
      ousgSharesAmount / OUSG_TO_ROUSG_SHARES_MULTIPLIER
    );
    emit Transfer(address(0), msg.sender, getROUSGByShares(ousgSharesAmount));
    emit TransferShares(_account, address(0), ousgSharesAmount);
  }

Impact: High, as token supply can be endlessly inflated and user tokens can be burned on demand

Likelihood: Low, as it requires a malicious or compromised admin/burner.

Give those roles only to contracts that have a Timelock mechanism so that users have enough time to exit their rOUSG positions if they decide that they don't agree with a transaction of the admin/burner.

2. Using an older Solidity compiler version

Description

Currently the protocol contracts use pragma solidity 0.8.16; which is a version that according to the List of Known Bugs in Solidity contains some Low severity issues. It is highly suggested to update the compiler version to a more recent one to make use of bugfixes and optimizations.

3. Outstanding USDC approvals during minting and OUSG approvals during redemption in OUSGInstantManager.sol contract

4. usdcReceiver in OUSGInstantManager.sol contract cannot be changed

GitHub Links

Description

The usdcReceiver address in the OUSGInstantManager.sol contract is immutable, meaning it cannot be changed after contract deployment. If this address gets compromised, there's no mechanism to update it, potentially leading to loss of funds as all USDC meant for the protocol would be sent to a malicious address.

  // The address that receives USDC for subscriptions
  address public immutable usdcReceiver;

Introduce a secure function to update the usdcReceiver address. This function should be guarded by appropriate access control measures to ensure only authorized parties can change the address. Also it would be helpful in this function to implement check if the the current usdcReceiver address have zero balance of USDC and only then to allow the changing of usdcReceiver address.

5. The minBUIDLRedeemAmount state variable in OUSGInstantManager.sol can be changed, which is completely unwanted behavuour by contest README

GitHub Links

Description

In the contest README write the following:

There is a BUIDL redemption minimum requirement that can assumed to be 250,000 BUIDL tokens that we do not to inherit for OUSG/rOUSG holders. To bypass this, we have added logic to ensure that the minimum amount of BUIDL that this contract redeems is always at least 250,000. As a consequence, there will sometimes be leftover USDC held inthe contract. We also have logic to use the USDC to cover the redemptions when the redemption amount is less than the contracts USDC balance.

The minBUIDLRedeemAmount is a critical parameter in the OUSGInstantManager.sol contract, intended to ensure that a minimum amount of BUIDL tokens is redeemed, aligning with the protocol's economic mechanisms and incentives. The ability to change this variable could lead to potential disruptions in the protocol's intended functionality, especially if set to values that diverge from the original design and intentions outlined in the project's documentation.

  // The minimum amount of BUIDL that must be redeemed in a single redemption
  // with the BUIDLRedeemer contract
  uint256 public minBUIDLRedeemAmount = 250_000e6;

If the minBUIDLRedeemAmount is meant to be a constant or only updated under very specific circumstances, consider implementing mechanisms to lock this value or strictly limit its mutability. This could involve using a governance process for changes or setting it as immutable if it truly should never change.

6. Change the getOUSGPrice() function to check if price is greater or equals to MINIMUM_OUSG_PRICE value

GitHub Links

Description

Change the getOUSGPrice() function to check if price is greater or equals to MINIMUM_OUSG_PRICE value for better understanding, because the specific for MINIMUM_OUSG_PRICE is: A uint256 constant set to 105e18 to act as a safety circuit breaker in case of Oracle malfunction. This ensures that the price of OUSG doesn't fall below this minimum threshold.

 function getOUSGPrice() public view returns (uint256 price) {
    (price, ) = oracle.getPriceData();
    require(
      price > MINIMUM_OUSG_PRICE,
      "OUSGInstantManager::getOUSGPrice: Price unexpectedly low"
    );
  }

Same for the checks and in setMintFee(), setRedeemFee() functions.

Modify the getOUSGPrice() function as following:

 function getOUSGPrice() public view returns (uint256 price) {
    (price, ) = oracle.getPriceData();
-   require(
-     price > MINIMUM_OUSG_PRICE,
-     "OUSGInstantManager::getOUSGPrice: Price unexpectedly low"
-   );
+  require(
+     price >= MINIMUM_OUSG_PRICE,
+     "OUSGInstantManager::getOUSGPrice: Price unexpectedly low"
+   );
  }

7. No default fees configuration (mintFee and redeemFee)

Description

The absence of default configurations for mintFee and redeemFee can lead to unpredictability in fee structures, potentially causing confusion among users or unintentionally zero fees if not set post-deployment.

Establish sensible default values for both mintFee and redeemFee within the contract, ensuring that fee structures are in place immediately upon deployment. Also include checks in the contract's initialization function to ensure that fees are explicitly set before the contract becomes operational.

8. Missing calls to base initializers in rUSDY

Description

The __rOUSG_init() function doesn't call the initializers for some of the base contracts:

  • Initializable
  • ContextUpgradeable
  • PausableUpgradeable
  • AccessControlEnumerableUpgradeable

Modify the __rOUSG_init() function to call the initializers of all base contracts. This may include manually invoking each base initializer or utilizing a tool like OpenZeppelin's Initializable pattern to streamline the process.


9. Consider bounding input array length

The functions below take in an unbounded array, and make function calls for entries in the array. While the function will revert if it eventually runs out of gas, it may be a nicer user experience to require() that the length of the array is below some reasonable maximum, so that the user doesn't have to use up a full transaction's gas only to see that the transaction reverts.

<details> <summary><i>There are 2 instances of this issue:</i></summary>
📁 File: contracts/ousg/ousgInstantManager.sol

804:     for (uint256 i = 0; i < exCallData.length; ++i) {
805:       (bool success, bytes memory ret) = address(exCallData[i].target).call{
806:         value: exCallData[i].value
807:       }(exCallData[i].data);
808:       require(success, "Call Failed");
809:       results[i] = ret;
810:     }

804

📁 File: contracts/ousg/rOUSGFactory.sol

/// @audit exCallData.length not bounded
125:     for (uint256 i = 0; i < exCallData.length; ++i) {
126:       (bool success, bytes memory ret) = address(exCallData[i].target).call{
127:         value: exCallData[i].value
128:       }(exCallData[i].data);
129:       require(success, "Call Failed");
130:       results[i] = ret;
131:     }

125

</details>

10. Constant decimal values

The use of fixed decimal values such as 1e18 or 1e8 in Solidity contracts can lead to inaccuracies, bugs, and vulnerabilities, particularly when interacting with tokens having different decimal configurations. Not all ERC20 tokens follow the standard 18 decimal places, and assumptions about decimal places can lead to miscalculations.

<details> <summary><i>There are 3 instances of this issue:</i></summary>
📁 File: contracts/ousg/ousgInstantManager.sol

689:     uint256 amountE36 = _scaleUp(usdcAmountIn) * 1e18;

704:     usdcOwed = _scaleDown(amountE36 / 1e18);

689, 704

📁 File: contracts/ousg/rOUSG.sol

367:       (_rOUSGAmount * 1e18 * OUSG_TO_ROUSG_SHARES_MULTIPLIER) / getOUSGPrice();

367

</details>

11. constructor/initialize function lacks parameter validation

Constructors and initialization functions play a critical role in contracts by setting important initial states when the contract is first deployed before the system starts. The parameters passed to the constructor and initialization functions directly affect the behavior of the contract / protocol. If incorrect parameters are provided, the system may fail to run, behave abnormally, be unstable, or lack security. Therefore, it's crucial to carefully check each parameter in the constructor and initialization functions. If an exception is found, the transaction should be rolled back.

<details> <summary><i>There are 3 instances of this issue:</i></summary>
📁 File: contracts/ousg/ousgInstantManager.sol

/// @audit defaultAdmin , rateLimiterConfig
144:   constructor(
145:     address defaultAdmin,
146:     address _usdc,
147:     address _usdcReciever,
148:     address _feeReceiver,
149:     address _ousgOracle,
150:     address _ousg,
151:     address _rousg,
152:     address _buidl,
153:     address _buidlRedeemer,
154:     RateLimiterConfig memory rateLimiterConfig
155:   )
156:     InstantMintTimeBasedRateLimiter(
157:       rateLimiterConfig.mintLimitDuration,
158:       rateLimiterConfig.redeemLimitDuration,
159:       rateLimiterConfig.mintLimit,
160:       rateLimiterConfig.redeemLimit
161:     )
162:   {

144

📁 File: contracts/ousg/rOUSG.sol

/// @audit _kycRegistry , requirementGroup , _ousg , guardian , _oracle
102:   function initialize(
103:     address _kycRegistry,
104:     uint256 requirementGroup,
105:     address _ousg,
106:     address guardian,
107:     address _oracle
108:   ) public virtual initializer {

102

📁 File: contracts/ousg/rOUSGFactory.sol

/// @audit _guardian
47:   constructor(address _guardian) {

47

</details>

Initialize function does not use the initializer modifier

The initialize() functions below are not called by another contract atomically after the contract is deployed, so it's possible for a malicious user to call initialize() which, if it's noticed in time, would require the project to re-deploy the contract in order to properly initialize. Consider creating a factory contract, which will new and initialize() each contract atomically.

<i>There is one instance of this issue:</i>

📁 File: contracts/ousg/rOUSG.sol

102:   function initialize(
103:     address _kycRegistry,
104:     uint256 requirementGroup,
105:     address _ousg,
106:     address guardian,
107:     address _oracle
108:   ) public virtual initializer {

102


12. require() should be used instead of assert()

Prior to solidity version 0.8.0, hitting an assert consumes the remainder of the transaction's available gas rather than returning it, as require()/revert() do. assert() should be avoided even past solidity version 0.8.0 as its documentation states that "The assert function creates an error of type Panic(uint256). ... Properly functioning code should never create a Panic, not even on invalid external input. If this happens, then there is a bug in your contract which you should fix".

<i>There is one instance of this issue:</i>

📁 File: contracts/ousg/rOUSGFactory.sol

94:     assert(rOUSGProxyAdmin.owner() == guardian);

94


13. Setters should prevent re-setting of the same value

This especially problematic when the setter also emits the same value, which may be confusing to offline parsers

<details> <summary><i>There are 6 instances of this issue:</i></summary>
📁 File: contracts/ousg/ousgInstantManager.sol

/// @note Confidence: 100.00%
554:   function setMintFee(
555:     uint256 _mintFee
556:   ) external override onlyRole(CONFIGURER_ROLE) {
557:     require(mintFee < 200, "OUSGInstantManager::setMintFee: Fee too high");
558:     emit MintFeeSet(mintFee, _mintFee);
559:     mintFee = _mintFee;
560:   }

/// @note Confidence: 100.00%
567:   function setRedeemFee(
568:     uint256 _redeemFee
569:   ) external override onlyRole(CONFIGURER_ROLE) {
570:     require(redeemFee < 200, "OUSGInstantManager::setRedeemFee: Fee too high");
571:     emit RedeemFeeSet(redeemFee, _redeemFee);
572:     redeemFee = _redeemFee;
573:   }

/// @note Confidence: 100.00%
622:   function setMinimumBUIDLRedemptionAmount(
623:     uint256 _minimumBUIDLRedemptionAmount
624:   ) external override onlyRole(DEFAULT_ADMIN_ROLE) {
625:     emit MinimumBUIDLRedemptionAmountSet(
626:       minBUIDLRedeemAmount,
627:       _minimumBUIDLRedemptionAmount
628:     );
629:     minBUIDLRedeemAmount = _minimumBUIDLRedemptionAmount;
630:   }

/// @note Confidence: 100.00%
638:   function setOracle(
639:     address _oracle
640:   ) external override onlyRole(DEFAULT_ADMIN_ROLE) {
641:     emit OracleSet(address(oracle), _oracle);
642:     oracle = IRWAOracle(_oracle);
643:   }

/// @note Confidence: 100.00%
663:   function setInvestorBasedRateLimiter(
664:     address _investorBasedRateLimiter
665:   ) external override onlyRole(DEFAULT_ADMIN_ROLE) {
666:     emit InvestorBasedRateLimiterSet(
667:       address(investorBasedRateLimiter),
668:       _investorBasedRateLimiter
669:     );
670:     investorBasedRateLimiter = IInvestorBasedRateLimiter(
671:       _investorBasedRateLimiter
672:     );
673:   }

554, 567, 622, 638, 663

📁 File: contracts/ousg/rOUSG.sol

/// @note Confidence: 100.00%
613:   function setOracle(address _oracle) external onlyRole(DEFAULT_ADMIN_ROLE) {
614:     emit OracleSet(address(oracle), _oracle);
615:     oracle = IRWAOracle(_oracle);
616:   }

613

</details>

14. Upgradeable contract is missing a __gap[50] storage variable at the end to allow for new storage variables in later versions

See this link for a description of this storage variable. While some contracts may not currently be sub-classed, adding the variable now protects against forgetting to add it in the future.

<i>There is one instance of this issue:</i>

📁 File: contracts/ousg/rOUSG.sol

55: contract ROUSG is
56:   Initializable,
57:   ContextUpgradeable,
58:   PausableUpgradeable,
59:   AccessControlEnumerableUpgradeable,
60:   KYCRegistryClientUpgradeable,

55


15. @openzeppelin/contracts should be upgraded to a newer version

These contracts import contracts from @openzeppelin/contracts, but they are not using the latest version.

❗ Issue is removed from: (pech, sme6en)

<details> <summary><i>There are 11 instances of this issue:</i></summary>
📁 File: contracts/ousg/ousgInstantManager.sol

18: import "contracts/external/openzeppelin/contracts/access/AccessControlEnumerable.sol";
19: import "contracts/external/openzeppelin/contracts/security/ReentrancyGuard.sol";
20: import "contracts/external/openzeppelin/contracts/token/IERC20Metadata.sol";

18

📁 File: contracts/ousg/rOUSG.sol

18: import "contracts/external/openzeppelin/contracts/token/IERC20.sol";
19: import "contracts/external/openzeppelin/contracts-upgradeable/token/ERC20/IERC20Upgradeable.sol";
20: import "contracts/external/openzeppelin/contracts-upgradeable/token/ERC20/IERC20MetadataUpgradeable.sol";
21: import "contracts/external/openzeppelin/contracts-upgradeable/proxy/Initializable.sol";
22: import "contracts/external/openzeppelin/contracts-upgradeable/utils/ContextUpgradeable.sol";
23: import "contracts/external/openzeppelin/contracts-upgradeable/security/PausableUpgradeable.sol";
24: import "contracts/external/openzeppelin/contracts-upgradeable/access/AccessControlEnumerableUpgradeable.sol";

18

📁 File: contracts/ousg/rOUSGFactory.sol

19: import "contracts/external/openzeppelin/contracts/proxy/ProxyAdmin.sol";

19

</details>

16. Add inline comments for unnamed variables

function foo(address x, address) -> function foo(address x, address /* y */)

<i>There is one instance of this issue:</i>

📁 File: contracts/ousg/rOUSG.sol

586:   function _beforeTokenTransfer(
587:     address from,
588:     address to,
589:     uint256
590:   ) internal view {

586


17. Consider using SafeTransferLib.safeTransferETH() or Address.sendValue() for clearer semantic meaning

These Functions indicate their purpose with their name more clearly than using low-level calls.

<details> <summary><i>There are 2 instances of this issue:</i></summary>
📁 File: contracts/ousg/ousgInstantManager.sol

805:       (bool success, bytes memory ret) = address(exCallData[i].target).call{
806:         value: exCallData[i].value
807:       }(exCallData[i].data);

805

📁 File: contracts/ousg/rOUSGFactory.sol

126:       (bool success, bytes memory ret) = address(exCallData[i].target).call{
127:         value: exCallData[i].value
128:       }(exCallData[i].data);

126

</details>

18. Contract uses both require()/revert() as well as custom errors

Consider using just one method in a single file

<i>There is one instance of this issue:</i>

📁 File: contracts/ousg/rOUSG.sol

55: contract ROUSG is

55


19. Custom error without details

Consider adding some parameters to the error to indicate which user or values caused the failure.

<i>There is one instance of this issue:</i>

📁 File: contracts/ousg/rOUSG.sol

90:   error UnwrapTooSmall();

90


20. Custom errors should be used rather than revert()/require()

Custom errors are available from solidity version 0.8.4. Custom errors are more easily processed in try-catch blocks, and are easier to re-use and maintain.

<details> <summary><i>There are 50 instances of this issue:</i></summary>
📁 File: contracts/ousg/ousgInstantManager.sol

163:     require(
164:       address(_usdc) != address(0),
165:       "OUSGInstantManager: USDC cannot be 0x0"
166:     );
167:     require(
168:       address(_usdcReciever) != address(0),
169:       "OUSGInstantManager: USDC Receiver cannot be 0x0"
170:     );
171:     require(
172:       address(_feeReceiver) != address(0),
173:       "OUSGInstantManager: feeReceiver cannot be 0x0"
174:     );
175:     require(
176:       address(_ousgOracle) != address(0),
177:       "OUSGInstantManager: OUSG Oracle cannot be 0x0"
178:     );
179:     require(_ousg != address(0), "OUSGInstantManager: OUSG cannot be 0x0");
180:     require(_rousg != address(0), "OUSGInstantManager: rOUSG cannot be 0x0");
181:     require(_buidl != address(0), "OUSGInstantManager: BUIDL cannot be 0x0");
182:     require(
183:       address(_buidlRedeemer) != address(0),
184:       "OUSGInstantManager: BUIDL Redeemer cannot be 0x0"
185:     );
186:     require(
187:       IERC20Metadata(_ousg).decimals() == IERC20Metadata(_rousg).decimals(),
188:       "OUSGInstantManager: OUSG decimals must be equal to USDC decimals"
189:     );
190:     require(
191:       IERC20Metadata(_usdc).decimals() == IERC20Metadata(_buidl).decimals(),
192:       "OUSGInstantManager: USDC decimals must be equal to BUIDL decimals"
193:     );

205:     require(
206:       OUSG_TO_ROUSG_SHARES_MULTIPLIER ==
207:         rousg.OUSG_TO_ROUSG_SHARES_MULTIPLIER(),
208:       "OUSGInstantManager: OUSG to rOUSG shares multiplier must be equal to rOUSG's"
209:     );

282:     require(
283:       IERC20Metadata(address(usdc)).decimals() == 6,
284:       "OUSGInstantManager::_mint: USDC decimals must be 6"
285:     );
286:     require(
287:       usdcAmountIn >= minimumDepositAmount,
288:       "OUSGInstantManager::_mint: Deposit amount too small"
289:     );

298:     require(
299:       usdc.allowance(msg.sender, address(this)) >= usdcAmountIn,
300:       "OUSGInstantManager::_mint: Allowance must be given to OUSGInstantManager"
301:     );

310:     require(
311:       ousgAmountOut > 0,
312:       "OUSGInstantManager::_mint: net mint amount can't be zero"
313:     );

344:     require(
345:       ousg.allowance(msg.sender, address(this)) >= ousgAmountIn,
346:       "OUSGInstantManager::redeem: Insufficient allowance"
347:     );

371:     require(
372:       rousg.allowance(msg.sender, address(this)) >= rousgAmountIn,
373:       "OUSGInstantManager::redeemRebasingOUSG: Insufficient allowance"
374:     );

391:     require(
392:       IERC20Metadata(address(usdc)).decimals() == 6,
393:       "OUSGInstantManager::_redeem: USDC decimals must be 6"
394:     );
395:     require(
396:       IERC20Metadata(address(buidl)).decimals() == 6,
397:       "OUSGInstantManager::_redeem: BUIDL decimals must be 6"
398:     );

402:     require(
403:       usdcAmountToRedeem >= minimumRedemptionAmount,
404:       "OUSGInstantManager::_redeem: Redemption amount too small"
405:     );

417:     require(
418:       usdcAmountOut > 0,
419:       "OUSGInstantManager::_redeem: redeem amount can't be zero"
420:     );

459:     require(
460:       buidl.balanceOf(address(this)) >= minBUIDLRedeemAmount,
461:       "OUSGInstantManager::_redeemBUIDL: Insufficient BUIDL balance"
462:     );

466:     require(
467:       usdc.balanceOf(address(this)) == usdcBalanceBefore + buidlAmountToRedeem,
468:       "OUSGInstantManager::_redeemBUIDL: BUIDL:USDC not 1:1"
469:     );

481:     require(
482:       price > MINIMUM_OUSG_PRICE,
483:       "OUSGInstantManager::getOUSGPrice: Price unexpectedly low"
484:     );

557:     require(mintFee < 200, "OUSGInstantManager::setMintFee: Fee too high");

570:     require(redeemFee < 200, "OUSGInstantManager::setRedeemFee: Fee too high");

584:     require(
585:       _minimumDepositAmount >= FEE_GRANULARITY,
586:       "setMinimumDepositAmount: Amount too small"
587:     );

602:     require(
603:       _minimumRedemptionAmount >= FEE_GRANULARITY,
604:       "setMinimumRedemptionAmount: Amount too small"
605:     );

653:     require(_feeReceiver != address(0), "FeeReceiver cannot be 0x0");

757:     require(!mintPaused, "OUSGInstantManager: Mint paused");

763:     require(!redeemPaused, "OUSGInstantManager: Redeem paused");

808:       require(success, "Call Failed");

163, 205, 282, 298, 310, 344, 371, 391, 402, 417, 459, 466, 481, 557, 570, 584, 602, 653, 757, 763, 808

📁 File: contracts/ousg/rOUSG.sol

282:     require(currentAllowance >= _amount, "TRANSFER_AMOUNT_EXCEEDS_ALLOWANCE");

333:     require(
334:       currentAllowance >= _subtractedValue,
335:       "DECREASED_ALLOWANCE_BELOW_ZERO"
336:     );

416:     require(_OUSGAmount > 0, "rOUSG: can't wrap zero OUSG tokens");

432:     require(_rOUSGAmount > 0, "rOUSG: can't unwrap zero rOUSG tokens");

477:     require(_owner != address(0), "APPROVE_FROM_ZERO_ADDRESS");
478:     require(_spender != address(0), "APPROVE_TO_ZERO_ADDRESS");

506:     require(_sender != address(0), "TRANSFER_FROM_THE_ZERO_ADDRESS");
507:     require(_recipient != address(0), "TRANSFER_TO_THE_ZERO_ADDRESS");

512:     require(
513:       _sharesAmount <= currentSenderShares,
514:       "TRANSFER_AMOUNT_EXCEEDS_BALANCE"
515:     );

533:     require(_recipient != address(0), "MINT_TO_THE_ZERO_ADDRESS");

558:     require(_account != address(0), "BURN_FROM_THE_ZERO_ADDRESS");

563:     require(_sharesAmount <= accountShares, "BURN_AMOUNT_EXCEEDS_BALANCE");

594:       require(_getKYCStatus(msg.sender), "rOUSG: 'sender' address not KYC'd");

599:       require(_getKYCStatus(from), "rOUSG: 'from' address not KYC'd");

604:       require(_getKYCStatus(to), "rOUSG: 'to' address not KYC'd");

282, 333, 416, 432, 477, 506, 512, 533, 558, 563, 594, 599, 604

📁 File: contracts/ousg/rOUSGFactory.sol

76:     require(!initialized, "ROUSGFactory: rOUSG already deployed");

129:       require(success, "Call Failed");

150:     require(msg.sender == guardian, "ROUSGFactory: You are not the Guardian");

76, 129, 150

</details>

22. Event emit should emit a parameter

Some emitted events do not have any emitted parameters. It is recommended to add some parameter such as state changes or value changes when events are emitted

<i>There are 4 instaces of this issue:</i>

📁 File: contracts/ousg/ousgInstantManager.sol

770:     emit MintPaused();

776:     emit MintUnpaused();

782:     emit RedeemPaused();

788:     emit RedeemUnpaused();

770, 776, 782, 788


22. Event is missing indexed fields

Index event fields make the field more quickly accessible to off-chain tools that parse events. However, note that each index field costs extra gas during emission, so it's not necessarily best to index the maximum allowed per event (three fields). Each event should use three indexed fields if there are three or more fields, and gas usage is not particularly of concern for the events in question. If there are fewer than three fields, all of the fields should be indexed.

<details> <summary><i>There are 2 instances of this issue:</i></summary>
📁 File: contracts/ousg/rOUSG.sol

149:   event TransferShares(
150:     address indexed from,
151:     address indexed to,
152:     uint256 sharesValue
153:   );

149

📁 File: contracts/ousg/rOUSGFactory.sol

141:   event rOUSGDeployed(
142:     address proxy,
143:     address proxyAdmin,
144:     address implementation,
145:     string name,
146:     string ticker
147:   );

141

</details>

23. Events are missing sender information

When an action is triggered based on a user's action, not being able to filter based on who triggered the action makes event processing a lot more cumbersome. Including the msg.sender the events of these types of action will make events much more useful to end users, especially when msg.sender is not tx.origin.

<details> <summary><i>There are 16 instances of this issue:</i></summary>
📁 File: contracts/ousg/ousgInstantManager.sol

558:     emit MintFeeSet(mintFee, _mintFee);

571:     emit RedeemFeeSet(redeemFee, _redeemFee);

589:     emit MinimumDepositAmountSet(minimumDepositAmount, _minimumDepositAmount);

606:     emit MinimumRedemptionAmountSet(
607:       minimumRedemptionAmount,
608:       _minimumRedemptionAmount
609:     );

625:     emit MinimumBUIDLRedemptionAmountSet(
626:       minBUIDLRedeemAmount,
627:       _minimumBUIDLRedemptionAmount
628:     );

641:     emit OracleSet(address(oracle), _oracle);

654:     emit FeeReceiverSet(feeReceiver, _feeReceiver);

666:     emit InvestorBasedRateLimiterSet(
667:       address(investorBasedRateLimiter),
668:       _investorBasedRateLimiter
669:     );

770:     emit MintPaused();

776:     emit MintUnpaused();

782:     emit RedeemPaused();

788:     emit RedeemUnpaused();

558, 571, 589, 606, 625, 641, 654, 666, 770, 776, 782, 788

📁 File: contracts/ousg/rOUSG.sol

457:     emit Transfer(_sender, _recipient, _amount);
458:     emit TransferShares(_sender, _recipient, _sharesToTransfer);

614:     emit OracleSet(address(oracle), _oracle);

639:     emit TransferShares(_account, address(0), ousgSharesAmount);

457, 614, 639

</details>

24. Events may be emitted out of order due to reentrancy

If a reentrancy occurs, some events may be emitted in an unexpected order, and this may be a problem if a third party expects a specific order for these events. Ensure that events follow the best practice of CEI.

<details> <summary><i>There are 8 instances of this issue:</i></summary>
📁 File: contracts/ousg/ousgInstantManager.sol

240:     emit InstantMintOUSG(msg.sender, usdcAmountIn, ousgAmountOut);

270:     emit InstantMintRebasingOUSG(
271:       msg.sender,
272:       usdcAmountIn,
273:       ousgAmountOut,
274:       rousgAmountOut
275:     );

240, 270

📁 File: contracts/ousg/rOUSG.sol

402:     emit TransferShares(msg.sender, _recipient, _sharesAmount);

404:     emit Transfer(msg.sender, _recipient, tokensAmount);

420:     emit Transfer(address(0), msg.sender, getROUSGByShares(ousgSharesAmount));
421:     emit TransferShares(address(0), msg.sender, ousgSharesAmount);

457:     emit Transfer(_sender, _recipient, _amount);
458:     emit TransferShares(_sender, _recipient, _sharesToTransfer);

402, 404, 420, 457

</details>

25. Events should be emitted before external calls

Ensure that events follow the best practice of check-effects-interaction, and are emitted before external calls.

<details> <summary><i>There are 14 instances of this issue:</i></summary>
📁 File: contracts/ousg/ousgInstantManager.sol

/// @audit transfer() on line 269
270:     emit InstantMintRebasingOUSG(
271:       msg.sender,
272:       usdcAmountIn,
273:       ousgAmountOut,
274:       rousgAmountOut
275:     );

/// @audit transferFrom() on line 319
321:     emit MintFeesDeducted(msg.sender, feeReceiver, usdcfees, usdcAmountIn);

/// @audit transferFrom() on line 348
350:     emit InstantRedemptionOUSG(msg.sender, ousgAmountIn, usdcAmountOut);

/// @audit unwrap() on line 376
380:     emit InstantRedemptionRebasingOUSG(
381:       msg.sender,
382:       rousgAmountIn,
383:       ousgAmountIn,
384:       usdcAmountOut
385:     );

/// @audit burn() on line 422
435:       emit MinimumBUIDLRedemption(
436:         msg.sender,
437:         minBUIDLRedeemAmount,
438:         usdcBalance + minBUIDLRedeemAmount - usdcAmountToRedeem
439:       );

/// @audit burn() on line 422
443:       emit BUIDLRedemptionSkipped(
444:         msg.sender,
445:         usdcAmountToRedeem,
446:         usdcBalance - usdcAmountToRedeem
447:       );

/// @audit transfer() on line 451
453:     emit RedeemFeesDeducted(msg.sender, feeReceiver, usdcFees, usdcAmountOut);

270, 321, 350, 380, 435, 443, 453

📁 File: contracts/ousg/rOUSG.sol

/// @audit transferFrom() on line 419
420:     emit Transfer(address(0), msg.sender, getROUSGByShares(ousgSharesAmount));
/// @audit transferFrom() on line 419
421:     emit TransferShares(address(0), msg.sender, ousgSharesAmount);

/// @audit transfer() on line 437
441:     emit Transfer(msg.sender, address(0), _rOUSGAmount);
/// @audit transfer() on line 437
442:     emit TransferShares(msg.sender, address(0), ousgSharesAmount);

/// @audit transfer() on line 634
638:     emit Transfer(address(0), msg.sender, getROUSGByShares(ousgSharesAmount));
/// @audit transfer() on line 634
639:     emit TransferShares(_account, address(0), ousgSharesAmount);

420, 441, 638

📁 File: contracts/ousg/rOUSGFactory.sol

/// @audit owner() on line 94
96:     emit rOUSGDeployed(
97:       address(rOUSGProxy),
98:       address(rOUSGProxyAdmin),
99:       address(rOUSGImplementation),
100:       rOUSGProxied.name(),
101:       rOUSGProxied.symbol()
102:     );

96

</details>

26. Events that mark critical parameter changes should contain both the old and the new value

This should especially be done if the new value is not required to be different from the old value

<details> <summary><i>There are 3 instances of this issue:</i></summary>
📁 File: contracts/ousg/ousgInstantManager.sol

638:   function setOracle(
639:     address _oracle
640:   ) external override onlyRole(DEFAULT_ADMIN_ROLE) {
641:     emit OracleSet(address(oracle), _oracle);
642:     oracle = IRWAOracle(_oracle);
643:   }

663:   function setInvestorBasedRateLimiter(
664:     address _investorBasedRateLimiter
665:   ) external override onlyRole(DEFAULT_ADMIN_ROLE) {
666:     emit InvestorBasedRateLimiterSet(
667:       address(investorBasedRateLimiter),
668:       _investorBasedRateLimiter
669:     );
670:     investorBasedRateLimiter = IInvestorBasedRateLimiter(
671:       _investorBasedRateLimiter
672:     );
673:   }

638, 663

📁 File: contracts/ousg/rOUSG.sol

613:   function setOracle(address _oracle) external onlyRole(DEFAULT_ADMIN_ROLE) {
614:     emit OracleSet(address(oracle), _oracle);
615:     oracle = IRWAOracle(_oracle);
616:   }

613

</details>

27. Multiple NatSpec Bugs/Improvements

NatSpec documentation for constructor is missing

<i>There is one instance of this issue:</i>

📁 File: contracts/ousg/rOUSGFactory.sol

47:   constructor(address _guardian) {

47


NatSpec: Constructor declarations should have @notice tags

<details> <summary><i>There are 2 instances of this issue:</i></summary>
📁 File: contracts/ousg/rOUSG.sol

98:   constructor() {

98

📁 File: contracts/ousg/rOUSGFactory.sol

47:   constructor(address _guardian) {

47

</details>

NatSpec: Contract declarations should have @author tags

<i>There is one instance of this issue:</i>

📁 File: contracts/ousg/rOUSG.sol

54:
55: contract ROUSG is
56:   Initializable,

54


NatSpec: Contract declarations should have @dev tags

<details> <summary><i>There are 2 instances of this issue:</i></summary>
📁 File: contracts/ousg/rOUSG.sol

54:
55: contract ROUSG is
56:   Initializable,

54

📁 File: contracts/ousg/rOUSGFactory.sol

36:  */
37: contract ROUSGFactory is IMulticall {
38:   bytes32 public constant DEFAULT_ADMIN_ROLE = bytes32(0);

36

</details>

NatSpec: Error declarations should have @notice tags

@notice is used to explain to end users what the error does, and the compiler interprets /// or /** comments as this tag if one was't explicitly provided

<i>There is one instance of this issue:</i>

📁 File: contracts/ousg/rOUSG.sol

90:   error UnwrapTooSmall();

90


NatSpec: Error declarations should have NatSpec descriptions

It is recommended that errors are fully annotated using NatSpec. It is clearly stated in the Solidity official documentation.

<i>There is one instance of this issue:</i>

📁 File: contracts/ousg/rOUSG.sol

90:   error UnwrapTooSmall();

90


NatSpec: Error missing NatSpec @dev tag

<i>There is one instance of this issue:</i>

📁 File: contracts/ousg/rOUSG.sol

89:   // Error when redeeming shares < `OUSG_TO_ROUSG_SHARES_MULTIPLIER`
90:   error UnwrapTooSmall();
91:

89


NatSpec: Event declarations should have @notice tags

@notice is used to explain to end users what the event emits, and the compiler interprets /// or /** comments as this tag if one was't explicitly provided

<i>There is one instance of this issue:</i>

📁 File: contracts/ousg/rOUSGFactory.sol

141:   event rOUSGDeployed(
142:     address proxy,
143:     address proxyAdmin,
144:     address implementation,
145:     string name,
146:     string ticker
147:   );

141


NatSpec: Event missing NatSpec @dev tag

<i>There is one instance of this issue:</i>

📁 File: contracts/ousg/rOUSG.sol

160:    */
161:   event OracleSet(address indexed oldOracle, address indexed newOracle);
162:

160


NatSpec: Event missing NatSpec @param tag

<details> <summary><i>There are 3 instances of this issue:</i></summary>
📁 File: contracts/ousg/rOUSG.sol

/// @audit Missing @param for all event parameters
149:   event TransferShares(
150:     address indexed from,
151:     address indexed to,
152:     uint256 sharesValue
153:   );

/// @audit Missing @param for all event parameters
161:   event OracleSet(address indexed oldOracle, address indexed newOracle);

149, 161

📁 File: contracts/ousg/rOUSGFactory.sol

/// @audit Missing @param for `proxyAdmin`, `name`, `ticker`
141:   event rOUSGDeployed(
142:     address proxy,
143:     address proxyAdmin,
144:     address implementation,
145:     string name,
146:     string ticker
147:   );

141

</details>

NatSpec: Function declarations should have @notice tags

@notice is used to explain to end users what the function does, and the compiler interprets /// or /** comments as this tag if one was't explicitly provided

<details> <summary><i>There are 19 instances of this issue:</i></summary>
📁 File: contracts/ousg/ousgInstantManager.sol

278:   function _mint(
279:     uint256 usdcAmountIn,
280:     address to
281:   ) internal returns (uint256 ousgAmountOut) {

388:   function _redeem(
389:     uint256 ousgAmountIn
390:   ) internal returns (uint256 usdcAmountOut) {

458:   function _redeemBUIDL(uint256 buidlAmountToRedeem) internal {

794:   function multiexcall(
795:     ExCallData[] calldata exCallData
796:   )
797:     external
798:     payable
799:     override
800:     onlyRole(DEFAULT_ADMIN_ROLE)
801:     returns (bytes[] memory results)
802:   {

278, 388, 458, 794

📁 File: contracts/ousg/rOUSG.sol

102:   function initialize(
103:     address _kycRegistry,
104:     uint256 requirementGroup,
105:     address _ousg,
106:     address guardian,
107:     address _oracle
108:   ) public virtual initializer {

112:   function __rOUSG_init(
113:     address _kycRegistry,
114:     uint256 requirementGroup,
115:     address _ousg,
116:     address guardian,
117:     address _oracle
118:   ) internal onlyInitializing {

128:   function __rOUSG_init_unchained(
129:     address _kycRegistry,
130:     uint256 _requirementGroup,
131:     address _ousg,
132:     address guardian,
133:     address _oracle
134:   ) internal onlyInitializing {

166:   function name() public pure returns (string memory) {

181:   function decimals() public pure returns (uint8) {

188:   function totalSupply() public view returns (uint256) {

356:   function sharesOf(address _account) public view returns (uint256) {

363:   function getSharesByROUSG(
364:     uint256 _rOUSGAmount
365:   ) public view returns (uint256) {

373:   function getROUSGByShares(uint256 _shares) public view returns (uint256) {

378:   function getOUSGPrice() public view returns (uint256 price) {

487:   function _sharesOf(address _account) internal view returns (uint256) {

642:   function pause() external onlyRole(PAUSER_ROLE) {

646:   function unpause() external onlyRole(DEFAULT_ADMIN_ROLE) {

650:   function setKYCRegistry(
651:     address registry
652:   ) external override onlyRole(CONFIGURER_ROLE) {

656:   function setKYCRequirementGroup(
657:     uint256 group
658:   ) external override onlyRole(CONFIGURER_ROLE) {

102, 112, 128, 166, 181, 188, 356, 363, 373, 378, 487, 642, 646, 650, 656

</details>

NatSpec: Function declarations should have NatSpec descriptions

It is recommended that Solidity contracts are fully annotated using NatSpec for all public interfaces (everything in the ABI). It is clearly stated in the Solidity official documentation. In complex projects such as DeFi, the interpretation of all functions and their arguments and returns is important for code readability and auditability.

<details> <summary><i>There are 12 instances of this issue:</i></summary>
📁 File: contracts/ousg/ousgInstantManager.sol

278:   function _mint(

388:   function _redeem(

458:   function _redeemBUIDL(uint256 buidlAmountToRedeem) internal {

794:   function multiexcall(

278, 388, 458, 794

📁 File: contracts/ousg/rOUSG.sol

102:   function initialize(

112:   function __rOUSG_init(

128:   function __rOUSG_init_unchained(

378:   function getOUSGPrice() public view returns (uint256 price) {

642:   function pause() external onlyRole(PAUSER_ROLE) {

646:   function unpause() external onlyRole(DEFAULT_ADMIN_ROLE) {

650:   function setKYCRegistry(

656:   function setKYCRequirementGroup(

102, 112, 128, 378, 642, 646, 650, 656

</details>

NatSpec: Functions missing NatSpec @dev tag

<details> <summary><i>There are 48 instances of this issue:</i></summary>
📁 File: contracts/ousg/ousgInstantManager.sol

277:
278:   function _mint(
279:     uint256 usdcAmountIn,

387:
388:   function _redeem(
389:     uint256 ousgAmountIn

457:
458:   function _redeemBUIDL(uint256 buidlAmountToRedeem) internal {
459:     require(

497:    */
498:   function setInstantMintLimit(
499:     uint256 _instantMintLimit

511:    */
512:   function setInstantRedemptionLimit(
513:     uint256 _instantRedemptionLimit

525:    */
526:   function setInstantMintLimitDuration(
527:     uint256 _instantMintLimitDuration

539:    */
540:   function setInstantRedemptionLimitDuration(
541:     uint256 _instantRedemptionLimitDuratioin

553:    */
554:   function setMintFee(
555:     uint256 _mintFee

566:    */
567:   function setRedeemFee(
568:     uint256 _redeemFee

580:    */
581:   function setMinimumDepositAmount(
582:     uint256 _minimumDepositAmount

598:    */
599:   function setMinimumRedemptionAmount(
600:     uint256 _minimumRedemptionAmount

621:    */
622:   function setMinimumBUIDLRedemptionAmount(
623:     uint256 _minimumBUIDLRedemptionAmount

637:    */
638:   function setOracle(
639:     address _oracle

649:    */
650:   function setFeeReceiver(
651:     address _feeReceiver

662:    */
663:   function setInvestorBasedRateLimiter(
664:     address _investorBasedRateLimiter

684:    */
685:   function _getMintAmount(
686:     uint256 usdcAmountIn,

698:    */
699:   function _getRedemptionAmount(
700:     uint256 ousgAmountBurned,

712:    */
713:   function _getInstantMintFees(
714:     uint256 usdcAmount

724:    */
725:   function _getInstantRedemptionFees(
726:     uint256 usdcAmount

767:   /// @notice Pause the mint functionality
768:   function pauseMint() external onlyRole(PAUSER_ROLE) {
769:     mintPaused = true;

773:   /// @notice Unpause the mint functionality
774:   function unpauseMint() external onlyRole(DEFAULT_ADMIN_ROLE) {
775:     mintPaused = false;

779:   /// @notice Pause the redeem functionality
780:   function pauseRedeem() external onlyRole(PAUSER_ROLE) {
781:     redeemPaused = true;

785:   /// @notice Unpause the redeem functionality
786:   function unpauseRedeem() external onlyRole(DEFAULT_ADMIN_ROLE) {
787:     redeemPaused = false;

793:   //////////////////////////////////////////////////////////////*/
794:   function multiexcall(
795:     ExCallData[] calldata exCallData

818:    */
819:   function retrieveTokens(
820:     address token,

277, 387, 457, 497, 511, 525, 539, 553, 566, 580, 598, 621, 637, 649, 662, 684, 698, 712, 724, 767, 773, 779, 785, 793, 818

📁 File: contracts/ousg/rOUSG.sol

97:   /// @custom:oz-upgrades-unsafe-allow constructor
98:   constructor() {
99:     _disableInitializers();

101:
102:   function initialize(
103:     address _kycRegistry,

111:
112:   function __rOUSG_init(
113:     address _kycRegistry,

127:
128:   function __rOUSG_init_unchained(
129:     address _kycRegistry,

165:    */
166:   function name() public pure returns (string memory) {
167:     return "Rebasing OUSG";

173:    */
174:   function symbol() public pure returns (string memory) {
175:     return "rOUSG";

180:    */
181:   function decimals() public pure returns (uint8) {
182:     return 18;

187:    */
188:   function totalSupply() public view returns (uint256) {
189:     return

301:    */
302:   function increaseAllowance(
303:     address _spender,

327:    */
328:   function decreaseAllowance(
329:     address _spender,

362:    */
363:   function getSharesByROUSG(
364:     uint256 _rOUSGAmount

372:    */
373:   function getROUSGByShares(uint256 _shares) public view returns (uint256) {
374:     return

377:
378:   function getOUSGPrice() public view returns (uint256 price) {
379:     (price, ) = oracle.getPriceData();

449:    */
450:   function _transfer(
451:     address _sender,

471:    */
472:   function _approve(
473:     address _owner,

486:    */
487:   function _sharesOf(address _account) internal view returns (uint256) {
488:     return shares[_account];

500:    */
501:   function _transferShares(
502:     address _sender,

528:    */
529:   function _mintShares(
530:     address _recipient,

641:
642:   function pause() external onlyRole(PAUSER_ROLE) {
643:     _pause();

645:
646:   function unpause() external onlyRole(DEFAULT_ADMIN_ROLE) {
647:     _unpause();

649:
650:   function setKYCRegistry(
651:     address registry

655:
656:   function setKYCRequirementGroup(
657:     uint256 group

97, 101, 111, 127, 165, 173, 180, 187, 301, 327, 362, 372, 377, 449, 471, 486, 500, 528, 641, 645, 649, 655

📁 File: contracts/ousg/rOUSGFactory.sol

46:
47:   constructor(address _guardian) {
48:     guardian = _guardian;

46

</details>

NatSpec: Functions missing NatSpec @param tag

It is recommended that Solidity contracts are fully annotated using NatSpec for all public interfaces (everything in the ABI). It is clearly stated in the Solidity official documentation. In complex projects such as DeFi, the interpretation of all functions and their arguments and returns is important for code readability and auditability.

<details> <summary><i>There are 54 instances of this issue:</i></summary>
📁 File: contracts/ousg/ousgInstantManager.sol

/// @audit Missing @param for `defaultAdmin`, `_usdcReciever`, `_feeReceiver`, `_ousgOracle`, `_buidlRedeemer`, `rateLimiterConfig`
144:   constructor(
145:     address defaultAdmin,
146:     address _usdc,
147:     address _usdcReciever,
148:     address _feeReceiver,
149:     address _ousgOracle,
150:     address _ousg,
151:     address _rousg,
152:     address _buidl,
153:     address _buidlRedeemer,
154:     RateLimiterConfig memory rateLimiterConfig
155:   )
156:     InstantMintTimeBasedRateLimiter(
157:       rateLimiterConfig.mintLimitDuration,
158:       rateLimiterConfig.redeemLimitDuration,
159:       rateLimiterConfig.mintLimit,
160:       rateLimiterConfig.redeemLimit
161:     )
162:   {

/// @audit Missing @param for all function parameters
230:   function mint(
231:     uint256 usdcAmountIn
232:   )
233:     external
234:     override
235:     nonReentrant
236:     whenMintNotPaused
237:     returns (uint256 ousgAmountOut)
238:   {

/// @audit Missing @param for all function parameters
254:   function mintRebasingOUSG(
255:     uint256 usdcAmountIn
256:   )
257:     external
258:     override
259:     nonReentrant
260:     whenMintNotPaused
261:     returns (uint256 rousgAmountOut)
262:   {

/// @audit Missing @param for all function parameters
278:   function _mint(
279:     uint256 usdcAmountIn,
280:     address to
281:   ) internal returns (uint256 ousgAmountOut) {

/// @audit Missing @param for all function parameters
335:   function redeem(
336:     uint256 ousgAmountIn
337:   )
338:     external
339:     override
340:     nonReentrant
341:     whenRedeemNotPaused
342:     returns (uint256 usdcAmountOut)
343:   {

/// @audit Missing @param for all function parameters
362:   function redeemRebasingOUSG(
363:     uint256 rousgAmountIn
364:   )
365:     external
366:     override
367:     nonReentrant
368:     whenRedeemNotPaused
369:     returns (uint256 usdcAmountOut)
370:   {

/// @audit Missing @param for all function parameters
388:   function _redeem(
389:     uint256 ousgAmountIn
390:   ) internal returns (uint256 usdcAmountOut) {

/// @audit Missing @param for all function parameters
458:   function _redeemBUIDL(uint256 buidlAmountToRedeem) internal {

/// @audit Missing @param for all function parameters
498:   function setInstantMintLimit(
499:     uint256 _instantMintLimit
500:   ) external override onlyRole(CONFIGURER_ROLE) {

/// @audit Missing @param for all function parameters
512:   function setInstantRedemptionLimit(
513:     uint256 _instantRedemptionLimit
514:   ) external override onlyRole(CONFIGURER_ROLE) {

/// @audit Missing @param for all function parameters
526:   function setInstantMintLimitDuration(
527:     uint256 _instantMintLimitDuration
528:   ) external override onlyRole(CONFIGURER_ROLE) {

/// @audit Missing @param for all function parameters
540:   function setInstantRedemptionLimitDuration(
541:     uint256 _instantRedemptionLimitDuratioin
542:   ) external override onlyRole(CONFIGURER_ROLE) {

/// @audit Missing @param for all function parameters
554:   function setMintFee(
555:     uint256 _mintFee
556:   ) external override onlyRole(CONFIGURER_ROLE) {

/// @audit Missing @param for all function parameters
567:   function setRedeemFee(
568:     uint256 _redeemFee
569:   ) external override onlyRole(CONFIGURER_ROLE) {

/// @audit Missing @param for all function parameters
581:   function setMinimumDepositAmount(
582:     uint256 _minimumDepositAmount
583:   ) external override onlyRole(CONFIGURER_ROLE) {

/// @audit Missing @param for all function parameters
599:   function setMinimumRedemptionAmount(
600:     uint256 _minimumRedemptionAmount
601:   ) external override onlyRole(CONFIGURER_ROLE) {

/// @audit Missing @param for all function parameters
622:   function setMinimumBUIDLRedemptionAmount(
623:     uint256 _minimumBUIDLRedemptionAmount
624:   ) external override onlyRole(DEFAULT_ADMIN_ROLE) {

/// @audit Missing @param for all function parameters
650:   function setFeeReceiver(
651:     address _feeReceiver
652:   ) external override onlyRole(DEFAULT_ADMIN_ROLE) {

/// @audit Missing @param for all function parameters
663:   function setInvestorBasedRateLimiter(
664:     address _investorBasedRateLimiter
665:   ) external override onlyRole(DEFAULT_ADMIN_ROLE) {

/// @audit Missing @param for `usdcAmountIn`
685:   function _getMintAmount(
686:     uint256 usdcAmountIn,
687:     uint256 price
688:   ) internal view returns (uint256 ousgAmountOut) {

/// @audit Missing @param for `ousgAmountBurned`
699:   function _getRedemptionAmount(
700:     uint256 ousgAmountBurned,
701:     uint256 price
702:   ) internal view returns (uint256 usdcOwed) {

/// @audit Missing @param for all function parameters
713:   function _getInstantMintFees(
714:     uint256 usdcAmount
715:   ) internal view returns (uint256) {

/// @audit Missing @param for all function parameters
725:   function _getInstantRedemptionFees(
726:     uint256 usdcAmount
727:   ) internal view returns (uint256) {

/// @audit Missing @param for all function parameters
737:   function _scaleUp(uint256 amount) internal view returns (uint256) {

/// @audit Missing @param for all function parameters
747:   function _scaleDown(uint256 amount) internal view returns (uint256) {

/// @audit Missing @param for all function parameters
794:   function multiexcall(
795:     ExCallData[] calldata exCallData
796:   )
797:     external
798:     payable
799:     override
800:     onlyRole(DEFAULT_ADMIN_ROLE)
801:     returns (bytes[] memory results)
802:   {

144, 230, 254, 278, 335, 362, 388, 458, 498, 512, 526, 540, 554, 567, 581, 599, 622, 650, 663, 685, 699, 713, 725, 737, 747, 794

📁 File: contracts/ousg/rOUSG.sol

/// @audit Missing @param for all function parameters
102:   function initialize(
103:     address _kycRegistry,
104:     uint256 requirementGroup,
105:     address _ousg,
106:     address guardian,
107:     address _oracle
108:   ) public virtual initializer {

/// @audit Missing @param for all function parameters
112:   function __rOUSG_init(
113:     address _kycRegistry,
114:     uint256 requirementGroup,
115:     address _ousg,
116:     address guardian,
117:     address _oracle
118:   ) internal onlyInitializing {

/// @audit Missing @param for all function parameters
128:   function __rOUSG_init_unchained(
129:     address _kycRegistry,
130:     uint256 _requirementGroup,
131:     address _ousg,
132:     address guardian,
133:     address _oracle
134:   ) internal onlyInitializing {

/// @audit Missing @param for all function parameters
199:   function balanceOf(address _account) public view returns (uint256) {

/// @audit Missing @param for all function parameters
220:   function transfer(address _recipient, uint256 _amount) public returns (bool) {

/// @audit Missing @param for all function parameters
231:   function allowance(
232:     address _owner,
233:     address _spender
234:   ) public view returns (uint256) {

/// @audit Missing @param for all function parameters
251:   function approve(address _spender, uint256 _amount) public returns (bool) {

/// @audit Missing @param for all function parameters
276:   function transferFrom(
277:     address _sender,
278:     address _recipient,
279:     uint256 _amount
280:   ) public returns (bool) {

/// @audit Missing @param for all function parameters
302:   function increaseAllowance(
303:     address _spender,
304:     uint256 _addedValue
305:   ) public returns (bool) {

/// @audit Missing @param for all function parameters
328:   function decreaseAllowance(
329:     address _spender,
330:     uint256 _subtractedValue
331:   ) public returns (bool) {

/// @audit Missing @param for all function parameters
356:   function sharesOf(address _account) public view returns (uint256) {

/// @audit Missing @param for all function parameters
363:   function getSharesByROUSG(
364:     uint256 _rOUSGAmount
365:   ) public view returns (uint256) {

/// @audit Missing @param for all function parameters
373:   function getROUSGByShares(uint256 _shares) public view returns (uint256) {

/// @audit Missing @param for all function parameters
397:   function transferShares(
398:     address _recipient,
399:     uint256 _sharesAmount
400:   ) public returns (uint256) {

/// @audit Missing @param for all function parameters
415:   function wrap(uint256 _OUSGAmount) external whenNotPaused {

/// @audit Missing @param for all function parameters
431:   function unwrap(uint256 _rOUSGAmount) external whenNotPaused {

/// @audit Missing @param for all function parameters
450:   function _transfer(
451:     address _sender,
452:     address _recipient,
453:     uint256 _amount
454:   ) internal {

/// @audit Missing @param for all function parameters
472:   function _approve(
473:     address _owner,
474:     address _spender,
475:     uint256 _amount
476:   ) internal whenNotPaused {

/// @audit Missing @param for all function parameters
487:   function _sharesOf(address _account) internal view returns (uint256) {

/// @audit Missing @param for all function parameters
501:   function _transferShares(
502:     address _sender,
503:     address _recipient,
504:     uint256 _sharesAmount
505:   ) internal whenNotPaused {

/// @audit Missing @param for all function parameters
529:   function _mintShares(
530:     address _recipient,
531:     uint256 _sharesAmount
532:   ) internal whenNotPaused returns (uint256) {

/// @audit Missing @param for all function parameters
554:   function _burnShares(
555:     address _account,
556:     uint256 _sharesAmount
557:   ) internal whenNotPaused returns (uint256) {

/// @audit Missing @param for all function parameters
586:   function _beforeTokenTransfer(
587:     address from,
588:     address to,
589:     uint256
590:   ) internal view {

/// @audit Missing @param for all function parameters
650:   function setKYCRegistry(
651:     address registry
652:   ) external override onlyRole(CONFIGURER_ROLE) {

/// @audit Missing @param for all function parameters
656:   function setKYCRequirementGroup(
657:     uint256 group
658:   ) external override onlyRole(CONFIGURER_ROLE) {

102, 112, 128, 199, 220, 231, 251, 276, 302, 328, 356, 363, 373, 397, 415, 431, 450, 472, 487, 501, 529, 554, 586, 650, 656

📁 File: contracts/ousg/rOUSGFactory.sol

/// @audit Missing @param for all function parameters
47:   constructor(address _guardian) {

/// @audit Missing @param for `kycRegistry`, `requirementGroup`
70:   function deployRebasingOUSG(
71:     address kycRegistry,
72:     uint256 requirementGroup,
73:     address ousg,
74:     address oracle
75:   ) external onlyGuardian returns (address, address, address) {

/// @audit Missing @param for all function parameters
121:   function multiexcall(
122:     ExCallData[] calldata exCallData
123:   ) external payable override onlyGuardian returns (bytes[] memory results) {

47, 70, 121

</details>

NatSpec: Functions missing NatSpec @return tag

It is recommended that Solidity contracts are fully annotated using NatSpec for all public interfaces (everything in the ABI). It is clearly stated in the Solidity official documentation. In complex projects such as DeFi, the interpretation of all functions and their arguments and returns is important for code readability and auditability.

<details> <summary><i>There are 19 instances of this issue:</i></summary>
📁 File: contracts/ousg/ousgInstantManager.sol

/// @audit Missing @return for all function parameters
230:   function mint(
231:     uint256 usdcAmountIn
232:   )
233:     external
234:     override
235:     nonReentrant
236:     whenMintNotPaused
237:     returns (uint256 ousgAmountOut)
238:   {

/// @audit Missing @return for all function parameters
254:   function mintRebasingOUSG(
255:     uint256 usdcAmountIn
256:   )
257:     external
258:     override
259:     nonReentrant
260:     whenMintNotPaused
261:     returns (uint256 rousgAmountOut)
262:   {

/// @audit Missing @return for all function parameters
278:   function _mint(
279:     uint256 usdcAmountIn,
280:     address to
281:   ) internal returns (uint256 ousgAmountOut) {

/// @audit Missing @return for all function parameters
335:   function redeem(
336:     uint256 ousgAmountIn
337:   )
338:     external
339:     override
340:     nonReentrant
341:     whenRedeemNotPaused
342:     returns (uint256 usdcAmountOut)
343:   {

/// @audit Missing @return for all function parameters
362:   function redeemRebasingOUSG(
363:     uint256 rousgAmountIn
364:   )
365:     external
366:     override
367:     nonReentrant
368:     whenRedeemNotPaused
369:     returns (uint256 usdcAmountOut)
370:   {

/// @audit Missing @return for all function parameters
388:   function _redeem(
389:     uint256 ousgAmountIn
390:   ) internal returns (uint256 usdcAmountOut) {

/// @audit Missing @return for all function parameters
685:   function _getMintAmount(
686:     uint256 usdcAmountIn,
687:     uint256 price
688:   ) internal view returns (uint256 ousgAmountOut) {

/// @audit Missing @return for all function parameters
699:   function _getRedemptionAmount(
700:     uint256 ousgAmountBurned,
701:     uint256 price
702:   ) internal view returns (uint256 usdcOwed) {

/// @audit Missing @return for all function parameters
713:   function _getInstantMintFees(
714:     uint256 usdcAmount
715:   ) internal view returns (uint256) {

/// @audit Missing @return for all function parameters
725:   function _getInstantRedemptionFees(
726:     uint256 usdcAmount
727:   ) internal view returns (uint256) {

/// @audit Missing @return for all function parameters
737:   function _scaleUp(uint256 amount) internal view returns (uint256) {

/// @audit Missing @return for all function parameters
747:   function _scaleDown(uint256 amount) internal view returns (uint256) {

/// @audit Missing @return for all function parameters
794:   function multiexcall(
795:     ExCallData[] calldata exCallData
796:   )
797:     external
798:     payable
799:     override
800:     onlyRole(DEFAULT_ADMIN_ROLE)
801:     returns (bytes[] memory results)
802:   {

230, 254, 278, 335, 362, 388, 685, 699, 713, 725, 737, 747, 794

📁 File: contracts/ousg/rOUSG.sol

/// @audit Missing @return for all function parameters
302:   function increaseAllowance(
303:     address _spender,
304:     uint256 _addedValue
305:   ) public returns (bool) {

/// @audit Missing @return for all function parameters
328:   function decreaseAllowance(
329:     address _spender,
330:     uint256 _subtractedValue
331:   ) public returns (bool) {

/// @audit Missing @return for all function parameters
378:   function getOUSGPrice() public view returns (uint256 price) {

/// @audit Missing @return for all function parameters
529:   function _mintShares(
530:     address _recipient,
531:     uint256 _sharesAmount
532:   ) internal whenNotPaused returns (uint256) {

/// @audit Missing @return for all function parameters
554:   function _burnShares(
555:     address _account,
556:     uint256 _sharesAmount
557:   ) internal whenNotPaused returns (uint256) {

302, 328, 378, 529, 554

📁 File: contracts/ousg/rOUSGFactory.sol

/// @audit Missing @return for all function parameters
121:   function multiexcall(
122:     ExCallData[] calldata exCallData
123:   ) external payable override onlyGuardian returns (bytes[] memory results) {

121

</details>

NatSpec: Modifier declarations should have NatSpec descriptions

It is recommended that modifiers are fully annotated using NatSpec.

<i>There is one instance of this issue:</i>

📁 File: contracts/ousg/rOUSGFactory.sol

149:   modifier onlyGuardian() {

149


NatSpec: Modifier missing NatSpec @dev tag

<details> <summary><i>There are 3 instances of this issue:</i></summary>
📁 File: contracts/ousg/ousgInstantManager.sol

755:   /// @notice Ensure that the mint functionality is not paused
756:   modifier whenMintNotPaused() {
757:     require(!mintPaused, "OUSGInstantManager: Mint paused");

761:   /// @notice Ensure that the redeem functionality is not paused
762:   modifier whenRedeemNotPaused() {
763:     require(!redeemPaused, "OUSGInstantManager: Redeem paused");

755, 761

📁 File: contracts/ousg/rOUSGFactory.sol

148:
149:   modifier onlyGuardian() {
150:     require(msg.sender == guardian, "ROUSGFactory: You are not the Guardian");

148

</details>

28. Non-external/public state variables should begin with an underscore

According to the Solidity Style Guide, Non-external/public state variables should begin with an <a href="https://docs.soliditylang.org/en/latest/style-guide.html#underscore-prefix-for-non-external-functions-and-variables">underscore</a>.

❗ Issue is removed from: (pech)

<i>There are 3 instaces of this issue:</i>

📁 File: contracts/ousg/rOUSG.sol

/// @audit _shares
72:   mapping(address => uint256) private shares;

/// @audit _allowances
75:   mapping(address => mapping(address => uint256)) private allowances;

/// @audit _totalShares
78:   uint256 private totalShares;

72, 75, 78


29. Not using the latest versions of project dependencies

Update the project dependencies to their latest versions wherever possible.

Use tools such as retire.js, npm audit, and yarn audit to confirm that no vulnerable dependencies remain.

DependencyCurrent VersionLatest Version
forge-std1.5.31.8.1
openzeppelin-contracts4.8.35.0.2

<i>There is one instance of this issue:</i>

📁 File: contracts/ousg/ousgInstantManager.sol

1: /**SPDX-License-Identifier: BUSL-1.1

1


30. Prefer skip over revert model in iteration

It is preferable to skip operations on an array index when a condition is not met rather than reverting the whole transaction as reverting can introduce the possiblity of malicous actors purposefully introducing array objects which fail conditional checks within for/while loops so group operations fail. As such it is recommended to simply skip such array indices over reverting unless there is a valid security or logic reason behind not doing so.

<details> <summary><i>There are 2 instances of this issue:</i></summary>
📁 File: contracts/ousg/ousgInstantManager.sol

/// @audit reverts on line: 808
804:     for (uint256 i = 0; i < exCallData.length; ++i) {

804

📁 File: contracts/ousg/rOUSGFactory.sol

/// @audit reverts on line: 129
125:     for (uint256 i = 0; i < exCallData.length; ++i) {

125

</details>

31. Use @inheritdoc for overridden functions

<details> <summary><i>There are 20 instances of this issue:</i></summary>
📁 File: contracts/ousg/ousgInstantManager.sol

230:   function mint(
231:     uint256 usdcAmountIn
232:   )
233:     external
234:     override
235:     nonReentrant
236:     whenMintNotPaused
237:     returns (uint256 ousgAmountOut)
238:   {

254:   function mintRebasingOUSG(
255:     uint256 usdcAmountIn
256:   )
257:     external
258:     override
259:     nonReentrant
260:     whenMintNotPaused
261:     returns (uint256 rousgAmountOut)
262:   {

335:   function redeem(
336:     uint256 ousgAmountIn
337:   )
338:     external
339:     override
340:     nonReentrant
341:     whenRedeemNotPaused
342:     returns (uint256 usdcAmountOut)
343:   {

362:   function redeemRebasingOUSG(
363:     uint256 rousgAmountIn
364:   )
365:     external
366:     override
367:     nonReentrant
368:     whenRedeemNotPaused
369:     returns (uint256 usdcAmountOut)
370:   {

498:   function setInstantMintLimit(
499:     uint256 _instantMintLimit
500:   ) external override onlyRole(CONFIGURER_ROLE) {

512:   function setInstantRedemptionLimit(
513:     uint256 _instantRedemptionLimit
514:   ) external override onlyRole(CONFIGURER_ROLE) {

526:   function setInstantMintLimitDuration(
527:     uint256 _instantMintLimitDuration
528:   ) external override onlyRole(CONFIGURER_ROLE) {

540:   function setInstantRedemptionLimitDuration(
541:     uint256 _instantRedemptionLimitDuratioin
542:   ) external override onlyRole(CONFIGURER_ROLE) {

554:   function setMintFee(
555:     uint256 _mintFee
556:   ) external override onlyRole(CONFIGURER_ROLE) {

567:   function setRedeemFee(
568:     uint256 _redeemFee
569:   ) external override onlyRole(CONFIGURER_ROLE) {

581:   function setMinimumDepositAmount(
582:     uint256 _minimumDepositAmount
583:   ) external override onlyRole(CONFIGURER_ROLE) {

599:   function setMinimumRedemptionAmount(
600:     uint256 _minimumRedemptionAmount
601:   ) external override onlyRole(CONFIGURER_ROLE) {

622:   function setMinimumBUIDLRedemptionAmount(
623:     uint256 _minimumBUIDLRedemptionAmount
624:   ) external override onlyRole(DEFAULT_ADMIN_ROLE) {

638:   function setOracle(
639:     address _oracle
640:   ) external override onlyRole(DEFAULT_ADMIN_ROLE) {

650:   function setFeeReceiver(
651:     address _feeReceiver
652:   ) external override onlyRole(DEFAULT_ADMIN_ROLE) {

663:   function setInvestorBasedRateLimiter(
664:     address _investorBasedRateLimiter
665:   ) external override onlyRole(DEFAULT_ADMIN_ROLE) {

794:   function multiexcall(
795:     ExCallData[] calldata exCallData
796:   )
797:     external
798:     payable
799:     override
800:     onlyRole(DEFAULT_ADMIN_ROLE)
801:     returns (bytes[] memory results)
802:   {

230, 254, 335, 362, 498, 512, 526, 540, 554, 567, 581, 599, 622, 638, 650, 663, 794

📁 File: contracts/ousg/rOUSG.sol

650:   function setKYCRegistry(
651:     address registry
652:   ) external override onlyRole(CONFIGURER_ROLE) {

656:   function setKYCRequirementGroup(
657:     uint256 group
658:   ) external override onlyRole(CONFIGURER_ROLE) {

650, 656

📁 File: contracts/ousg/rOUSGFactory.sol

121:   function multiexcall(
122:     ExCallData[] calldata exCallData
123:   ) external payable override onlyGuardian returns (bytes[] memory results) {

121

</details>

32. Use of override is unnecessary

Starting with Solidity version 0.8.8, using the override keyword when the function solely overrides an interface function, and the function doesn't exist in multiple base contracts, is unnecessary.

<details> <summary><i>There are 20 instances of this issue:</i></summary>
📁 File: contracts/ousg/ousgInstantManager.sol

230:   function mint(
231:     uint256 usdcAmountIn
232:   )
233:     external
234:     override
235:     nonReentrant
236:     whenMintNotPaused
237:     returns (uint256 ousgAmountOut)
238:   {

254:   function mintRebasingOUSG(
255:     uint256 usdcAmountIn
256:   )
257:     external
258:     override
259:     nonReentrant
260:     whenMintNotPaused
261:     returns (uint256 rousgAmountOut)
262:   {

335:   function redeem(
336:     uint256 ousgAmountIn
337:   )
338:     external
339:     override
340:     nonReentrant
341:     whenRedeemNotPaused
342:     returns (uint256 usdcAmountOut)
343:   {

362:   function redeemRebasingOUSG(
363:     uint256 rousgAmountIn
364:   )
365:     external
366:     override
367:     nonReentrant
368:     whenRedeemNotPaused
369:     returns (uint256 usdcAmountOut)
370:   {

498:   function setInstantMintLimit(
499:     uint256 _instantMintLimit
500:   ) external override onlyRole(CONFIGURER_ROLE) {

512:   function setInstantRedemptionLimit(
513:     uint256 _instantRedemptionLimit
514:   ) external override onlyRole(CONFIGURER_ROLE) {

526:   function setInstantMintLimitDuration(
527:     uint256 _instantMintLimitDuration
528:   ) external override onlyRole(CONFIGURER_ROLE) {

540:   function setInstantRedemptionLimitDuration(
541:     uint256 _instantRedemptionLimitDuratioin
542:   ) external override onlyRole(CONFIGURER_ROLE) {

554:   function setMintFee(
555:     uint256 _mintFee
556:   ) external override onlyRole(CONFIGURER_ROLE) {

567:   function setRedeemFee(
568:     uint256 _redeemFee
569:   ) external override onlyRole(CONFIGURER_ROLE) {

581:   function setMinimumDepositAmount(
582:     uint256 _minimumDepositAmount
583:   ) external override onlyRole(CONFIGURER_ROLE) {

599:   function setMinimumRedemptionAmount(
600:     uint256 _minimumRedemptionAmount
601:   ) external override onlyRole(CONFIGURER_ROLE) {

622:   function setMinimumBUIDLRedemptionAmount(
623:     uint256 _minimumBUIDLRedemptionAmount
624:   ) external override onlyRole(DEFAULT_ADMIN_ROLE) {

638:   function setOracle(
639:     address _oracle
640:   ) external override onlyRole(DEFAULT_ADMIN_ROLE) {

650:   function setFeeReceiver(
651:     address _feeReceiver
652:   ) external override onlyRole(DEFAULT_ADMIN_ROLE) {

663:   function setInvestorBasedRateLimiter(
664:     address _investorBasedRateLimiter
665:   ) external override onlyRole(DEFAULT_ADMIN_ROLE) {

794:   function multiexcall(
795:     ExCallData[] calldata exCallData
796:   )
797:     external
798:     payable
799:     override
800:     onlyRole(DEFAULT_ADMIN_ROLE)
801:     returns (bytes[] memory results)
802:   {

230, 254, 335, 362, 498, 512, 526, 540, 554, 567, 581, 599, 622, 638, 650, 663, 794

📁 File: contracts/ousg/rOUSG.sol

650:   function setKYCRegistry(
651:     address registry
652:   ) external override onlyRole(CONFIGURER_ROLE) {

656:   function setKYCRequirementGroup(
657:     uint256 group
658:   ) external override onlyRole(CONFIGURER_ROLE) {

650, 656

📁 File: contracts/ousg/rOUSGFactory.sol

121:   function multiexcall(
122:     ExCallData[] calldata exCallData
123:   ) external payable override onlyGuardian returns (bytes[] memory results) {

121

</details>

33. Variables need not be initialized to zero

The default value for variables is zero, so initializing them to zero is superfluous.

<details> <summary><i>There are 4 instances of this issue:</i></summary>
📁 File: contracts/ousg/ousgInstantManager.sol

99:   uint256 public mintFee = 0;

102:   uint256 public redeemFee = 0;

804:     for (uint256 i = 0; i < exCallData.length; ++i) {

99, 102, 804

📁 File: contracts/ousg/rOUSGFactory.sol

125:     for (uint256 i = 0; i < exCallData.length; ++i) {

125

</details>

34.

Impact

OUSG & rOUSG tokens can not be instant minted under certain condition

Proof of Concept

OUSG & rOUSG tokens can not be instant minted under certain situation: 1. OUSGInstantManager.sol#minimumDepositAmount = 60_000e6; InstantMintTimeBasedRateLimiter.sol#instantMintLimit = 50_000e6; 2. Alice tries to mint (deposit) 60_000e6, but the mint function will revert, because in InstantMintTimeBasedRateLimiter.sol#_checkAndUpdateInstantMintLimit() function this check will revert (60_000e6 < 50_000e6):

    require(
      amount <= instantMintLimit - currentInstantMintAmount,
      "RateLimit: Mint exceeds rate limit"
    );

The problem cause when OUSGInstantManager.sol#minimumDepositAmount is greater than InstantMintTimeBasedRateLimiter.sol#instantMintLimit.

The exact same issue exists and in redemption process.

Important to note that this situation and the issue overall is different from the instant mint/redeem flow.

Tools Used

Manual Review

During the changing of instantMintLimit and instantRedemptionLimit doesn't allow to can be set with values smaller than minimumDepositAmount and minimumRedemptionAmount.

#0 - c4-pre-sort

2024-04-05T04:44:36Z

0xRobocop marked the issue as sufficient quality report

#1 - radeveth

2024-04-10T23:16:31Z

Hi, @3docSec! Please review my QA report again because I think it should be classified as grade-a report. Even this I think should be selected for report. Let's compare my report and the currently selected for report (https://github.com/code-423n4/2024-03-ondo-finance-findings/issues/162). The currently selected has 5 low and 5 NC issues. My QA report has 15 low issues (10 quality one, into protocol logic and 5 bot issues) and 18 NC issues. I also have a few medium issues downgraded to low.

#2 - 3docSec

2024-04-12T07:35:40Z

Hi @radeveth sorry it didn't get a grade before.

I disagree with selecting for report as a few findings are questionable at best: 3. is empty 9. is pointless because the array comes from the input 14. is plain wrong, rOUSG is not meant to be inherited so it doesn't need storage gap 17. makes no sense, because there is a calldata passed in the call 24 (especially) and 25 make little sense 34. is the only valuable one on top of what a bot would report

grade-b seems appropriate: a thorough bot-like report with one proper QA finding

#3 - c4-judge

2024-04-12T07:35:45Z

3docSec marked the issue as grade-b

#4 - radeveth

2024-04-12T10:52:31Z

#5 - 3docSec

2024-04-15T10:29:37Z

Multiple grade-b's don't necessarily make a grade-a I am afraid

#6 - radeveth

2024-04-15T10:32:30Z

but this multiple downgraded to QA mediums + all of my issues in the QA report make the whole report grade a.

#7 - radeveth

2024-04-15T10:36:02Z

also a reminder that the selected for report one currently has 5 low and 5 NC issues. look at my report and see how many problems I have. at least 13 of them are related to protocol logic others are from the bot, but this does not diminish their validity

AuditHub

A portfolio for auditors, a security profile for protocols, a hub for web3 security.

Built bymalatrax © 2024

Auditors

Browse

Contests

Browse

Get in touch

ContactTwitter