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
Rank: 9/72
Findings: 3
Award: $563.06
🌟 Selected for report: 0
🚀 Solo Findings: 0
490.6256 USDC - $490.63
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.
minimumDepositAmount
is 60,000 USDC, and the minimumRedemptionAmount
is 50,000 OUSG.OUSGInstantManager.sol#_mint()
functionfunction _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); }
OUSGInstantManager.sol#_redeem_()
functionfunction _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); }
Manual Review
I would suggest two things:
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.
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; }
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
64.1515 USDC - $64.15
https://github.com/code-423n4/2024-03-ondo-finance/blob/main/contracts/ousg/rOUSG.sol#L624-L640
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.
/** * @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"); } }
burn()
function: https://github.com/code-423n4/2024-03-ondo-finance/blob/main/contracts/ousg/rOUSG.sol#L619-L640_burnShares()
function: https://github.com/code-423n4/2024-03-ondo-finance/blob/main/contracts/ousg/rOUSG.sol#L554-L570_beforeTokenTransfer()
function: https://github.com/code-423n4/2024-03-ondo-finance/blob/main/contracts/ousg/rOUSG.sol#L586-L606It 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:
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
.
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
🌟 Selected for report: immeas
Also found by: 0xAkira, 0xCiphky, 0xGreyWolf, 0xJaeger, 0xMosh, 0xabhay, 0xlemon, 0xmystery, 0xweb3boy, Aamir, Abdessamed, Aymen0909, Breeje, DanielArmstrong, DarkTower, Dots, EaglesSecurity, FastChecker, HChang26, Honour, IceBear, JC, K42, Krace, MaslarovK, Omik, OxTenma, SAQ, Shubham, Stormreckson, Tigerfrake, Tychai0s, VAD37, ZanyBonzy, albahaca, arnie, ast3ros, asui, b0g0, bareli, baz1ka, btk, caglankaan, carrotsmuggler, cheatc0d3, dd0x7e8, grearlake, igbinosuneric, jaydhales, kaden, kartik_giri_47538, m4ttm, ni8mare, niser93, nonn_ac, oualidpro, pfapostol, pkqs90, popeye, radev_sw, samuraii77, slvDev, zabihullahazadzoi
8.2807 USDC - $8.28
№ | Title | Severity |
---|---|---|
1 | Burning is centralized | Low |
2 | Using an older Solidity compiler version | Low |
3 | Outstanding USDC approvals during minting and OUSG approvals during redemption in OUSGInstantManager.sol contract | Low |
4 | usdcReceiver in OUSGInstantManager.sol contract cannot be changed | Low |
5 | The minBUIDLRedeemAmount state variable in OUSGInstantManager.sol can be changed, which is completely unwanted behaviour by contest README | Low |
6 | Change the getOUSGPrice() function to check if price is greater or equals to MINIMUM_OUSG_PRICE value | Low |
7 | No default fees configuration (mintFee and redeemFee ) | Low |
8 | Missing calls to base initializers in rUSDY | Low |
9 | Consider bounding input array length | Low |
10 | Constant decimal values | Low |
11 | constructor /initialize function lacks parameter validation | Low |
12 | require() should be used instead of assert() | Low |
13 | Setters should prevent re-setting of the same value | Low |
14 | Upgradeable contract is missing a __gap[50] storage variable at the end to allow for new storage variables in later versions | Low |
15 | @openzeppelin/contracts should be upgraded to a newer version | Low |
16 | Add inline comments for unnamed variables | NC |
17 | Consider using SafeTransferLib.safeTransferETH() or Address.sendValue() for clearer semantic meaning | NC |
18 | Contract uses both require() /revert() as well as custom errors | NC |
19 | Custom error without details | NC |
20 | Custom errors should be used rather than revert() /require() | NC |
21 | Event emit should emit a parameter | NC |
22 | Event is missing indexed fields | NC |
23 | Events are missing sender information | NC |
24 | Events may be emitted out of order due to reentrancy | NC |
25 | Events should be emitted before external calls | NC |
26 | Events that mark critical parameter changes should contain both the old and the new value | NC |
27 | Multiple NatSpec Bugs/Improvements | NC |
28 | Non-external /public state variables should begin with an underscore | NC |
29 | Not using the latest versions of project dependencies | NC |
30 | Prefer skip over revert model in iteration | NC |
31 | Use @inheritdoc for overridden functions | NC |
32 | Use of override is unnecessary | NC |
33 | Variables need not be initialized to zero | NC |
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.
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.
USDC
approvals during minting and OUSG
approvals during redemption in OUSGInstantManager.sol
contractusdcReceiver
in OUSGInstantManager.sol
contract cannot be changedThe 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.
minBUIDLRedeemAmount
state variable in OUSGInstantManager.sol
can be changed, which is completely unwanted behavuour by contest READMEIn 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.
getOUSGPrice()
function to check if price
is greater or equals to MINIMUM_OUSG_PRICE
valueChange 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" + ); }
mintFee
and redeemFee
)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.
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.
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.
📁 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: }
</details>📁 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: }
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);
</details>📁 File: contracts/ousg/rOUSG.sol 367: (_rOUSGAmount * 1e18 * OUSG_TO_ROUSG_SHARES_MULTIPLIER) / getOUSGPrice();
constructor
/initialize
function lacks parameter validationConstructors 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: {
📁 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 {
</details>📁 File: contracts/ousg/rOUSGFactory.sol /// @audit _guardian 47: constructor(address _guardian) {
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 {
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);
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: }
</details>📁 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: }
__gap[50]
storage variable at the end to allow for new storage variables in later versionsSee 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,
@openzeppelin/contracts
should be upgraded to a newer versionThese contracts import contracts from @openzeppelin/contracts
, but they are not using the latest version.
<details> <summary><i>There are 11 instances of this issue:</i></summary>❗ Issue is removed from: (pech, sme6en)
📁 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";
📁 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";
</details>📁 File: contracts/ousg/rOUSGFactory.sol 19: import "contracts/external/openzeppelin/contracts/proxy/ProxyAdmin.sol";
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 {
SafeTransferLib.safeTransferETH()
or Address.sendValue()
for clearer semantic meaningThese 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);
</details>📁 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);
require()
/revert()
as well as custom errorsConsider 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
error
without detailsConsider 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();
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
</details>📁 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");
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();
indexed
fieldsIndex 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: );
</details>📁 File: contracts/ousg/rOUSGFactory.sol 141: event rOUSGDeployed( 142: address proxy, 143: address proxyAdmin, 144: address implementation, 145: string name, 146: string ticker 147: );
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
.
📁 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
</details>📁 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);
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: );
</details>📁 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);
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);
</details>📁 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: );
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: }
</details>📁 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: }
constructor
is missing<i>There is one instance of this issue:</i>
📁 File: contracts/ousg/rOUSGFactory.sol 47: constructor(address _guardian) {
@notice
tags📁 File: contracts/ousg/rOUSG.sol 98: constructor() {
</details>📁 File: contracts/ousg/rOUSGFactory.sol 47: constructor(address _guardian) {
@author
tags<i>There is one instance of this issue:</i>
📁 File: contracts/ousg/rOUSG.sol 54: 55: contract ROUSG is 56: Initializable,
@dev
tags📁 File: contracts/ousg/rOUSG.sol 54: 55: contract ROUSG is 56: Initializable,
</details>📁 File: contracts/ousg/rOUSGFactory.sol 36: */ 37: contract ROUSGFactory is IMulticall { 38: bytes32 public constant DEFAULT_ADMIN_ROLE = bytes32(0);
@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();
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();
@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:
@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: );
@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:
@param
tag📁 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);
</details>📁 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: );
@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
📁 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: {
📁 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>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(
📁 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>@dev
tag📁 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
</details>📁 File: contracts/ousg/rOUSGFactory.sol 46: 47: constructor(address _guardian) { 48: guardian = _guardian;
@param
tagIt 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
</details>📁 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) {
@return
tagIt 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) {
</details>📁 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) {
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() {
@dev
tag📁 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");
</details>📁 File: contracts/ousg/rOUSGFactory.sol 148: 149: modifier onlyGuardian() { 150: require(msg.sender == guardian, "ROUSGFactory: You are not the Guardian");
external
/public
state variables should begin with an underscoreAccording 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;
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.
Dependency | Current Version | Latest Version |
---|---|---|
forge-std | 1.5.3 | 1.8.1 |
openzeppelin-contracts | 4.8.3 | 5.0.2 |
<i>There is one instance of this issue:</i>
📁 File: contracts/ousg/ousgInstantManager.sol 1: /**SPDX-License-Identifier: BUSL-1.1
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) {
</details>📁 File: contracts/ousg/rOUSGFactory.sol /// @audit reverts on line: 129 125: for (uint256 i = 0; i < exCallData.length; ++i) {
@inheritdoc
for overridden functions📁 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) {
</details>📁 File: contracts/ousg/rOUSGFactory.sol 121: function multiexcall( 122: ExCallData[] calldata exCallData 123: ) external payable override onlyGuardian returns (bytes[] memory results) {
override
is unnecessaryStarting 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) {
</details>📁 File: contracts/ousg/rOUSGFactory.sol 121: function multiexcall( 122: ExCallData[] calldata exCallData 123: ) external payable override onlyGuardian returns (bytes[] memory results) {
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) {
</details>📁 File: contracts/ousg/rOUSGFactory.sol 125: for (uint256 i = 0; i < exCallData.length; ++i) {
OUSG
& rOUSG
tokens can not be instant minted under certain condition
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.
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
@3docSec
Please keep in mind that I have several mediums downgraded to valid QA issues.
So I really think my report deserves grade-a
!
#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