Ondo Finance - 0x6980's results

Report

Low Risk Issues

Total: 2 instances over 2 issues.

Non-critical Issues

Total: 841 instances over 94 issues.

Low Risk Issues

<a name="L-01"></a>[L-01] Using zero as a parameter

Taking 0 as a valid argument in Solidity without checks can lead to severe security issues. A historical example is the infamous 0x0 address bug where numerous tokens were lost. This happens because 0 can be interpreted as an uninitialized address, leading to transfers to the 0x0 address, effectively burning tokens. Moreover, 0 as a denominator in division operations would cause a runtime exception. It's also often indicative of a logical error in the caller's code. It's important to always validate input and handle edge cases like 0 appropriately. Use require() statements to enforce conditions and provide clear error messages to facilitate debugging and safer code.

Instances (1):

File: contracts/bridge/DestinationBridge.sol

/// @audit 1 parameter
139:           new address[](0)

139

<a name="L-02"></a>[L-02] Re-org attack

Re-orgs can happen in all EVM chains. In ethereum, where currently Frankencoin is deployed, it is not “super common” but it still happens, being the last one less than a year ago: ethereum-beacon-chain-blockchain-reorg The issue increases the changes of happening if deploying on L2’s/ rollups. where re-orgs have been much more active: polygon-hit-by-157-block-reorg-despite-hard-fork-to-reduce-reorgs being the last one, less than a year ago. Deploy the cloned Contract via create2 with a specific salt that includes msg.sender and address _existing

Instances (3):

File: contracts/usdy/rUSDYFactory.sol

82:     rUSDYImplementation = new rUSDY();

83:     rUSDYProxyAdmin = new ProxyAdmin();

84:     rUSDYProxy = new TokenProxy(
85:          address(rUSDYImplementation),
86:          address(rUSDYProxyAdmin),
87:          ""
88:        );

82, 83, 84-88

<a name="L-03"></a>[L-03] 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"

Instances (1):

File: contracts/usdy/rUSDYFactory.sol

100:     assert(rUSDYProxyAdmin.owner() == guardian);

100

<a name="L-04"></a>[L-04] TransferOwnership Should Be Two Step

Recommend considering implementing a two step process where the owner or admin nominates an account and the nominated account needs to call an acceptOwnership() function for the transfer of ownership to fully succeed. This ensures the nominated EOA account is a valid and active account.

Instances (2):

File: contracts/bridge/DestinationBridge.sol

71:     _transferOwnership(_owner);

71

File: contracts/bridge/SourceBridge.sol

49:     _transferOwnership(owner);

49

Non-critical Issues

<a name="N-11"></a>[N-11] Use Ownable2Step rather than Ownable

Ownable2Step and Ownable2StepUpgradeable prevent the contract ownership from mistakenly being transferred to an address that cannot handle it (e.g. due to a typo in the address), by requiring that the recipient of the owner permissions actively accept via a contract call of its own.

Instances (2):

File: contracts/bridge/DestinationBridge.sol

27: contract DestinationBridge is
28:      AxelarExecutable,
29:      MintTimeBasedRateLimiter,
30:      Ownable,
31:      Pausable

27-31

File: contracts/bridge/SourceBridge.sol

11: contract SourceBridge is Ownable, Pausable, IMulticall {

11

<a name="N-12"></a>[N-12] Consider implementing two-step procedure for updating protocol addresses

A copy-paste error or a typo may end up bricking protocol functionality, or sending tokens to an address with no known private key. Consider implementing a two-step procedure for updating protocol addresses, where the recipient is set as pending, and must 'accept' the assignment by making an affirmative call. A straight forward way of doing this would be to have the target contracts implement EIP-165, and to have the 'set' functions ensure that the recipient is of the right interface type.

Instances (5):

File: contracts/bridge/DestinationBridge.sol

210:   function addApprover(address approver) external onlyOwner {
211:         approvers[approver] = true;
212:         emit ApproverAdded(approver);
213:       }

210-213

File: contracts/usdy/rUSDY.sol

662:   function setOracle(address _oracle) external onlyRole(USDY_MANAGER_ROLE) {
663:         oracle = IRWADynamicOracle(_oracle);
664:       }

698:   function setBlocklist(
699:         address blocklist
700:       ) external override onlyRole(LIST_CONFIGURER_ROLE) {
701:         _setBlocklist(blocklist);
702:       }

709:   function setAllowlist(
710:         address allowlist
711:       ) external override onlyRole(LIST_CONFIGURER_ROLE) {
712:         _setAllowlist(allowlist);
713:       }

720:   function setSanctionsList(
721:         address sanctionsList
722:       ) external override onlyRole(LIST_CONFIGURER_ROLE) {
723:         _setSanctionsList(sanctionsList);
724:       }

662-664, 698-702, 709-713, 720-724

<a name="N-01"></a>[N-01] Adding a return statement when the function defines a named return variable, is redundant

Once the return variable has been assigned (or has its default value), there is no need to explicitly return it at the end of the function, since it's returned automatically

Instances (1):

File: contracts/rwaOracles/RWADynamicOracle.sol


267:     return

267

<a name="N-02"></a>[N-02] assembly blocks should have extensive comments

Assembly blocks are take a lot more time to audit than normal Solidity code, and often have gotchas and side-effects that the Solidity versions of the same code do not. Consider adding more comments explaining what is being done in every step of the assembly code

Instances (1):

File: contracts/rwaOracles/RWADynamicOracle.sol

350:     assembly {
351:           switch x
352:           case 0 {
353:             switch n
354:             case 0 {
355:               z := base
356:             }
357:             default {
358:               z := 0
359:             }
360:           }
361:           default {
362:             switch mod(n, 2)
363:             case 0 {
364:               z := base
365:             }
366:             default {
367:               z := x
368:             }
369:             let half := div(base, 2) // for rounding.
370:             for {
371:               n := div(n, 2)
372:             } n {
373:               n := div(n, 2)
374:             } {
375:               let xx := mul(x, x)
376:               if iszero(eq(div(xx, x), x)) {
377:                 revert(0, 0)
378:               }
379:               let xxRound := add(xx, half)
380:               if lt(xxRound, xx) {
381:                 revert(0, 0)
382:               }
383:               x := div(xxRound, base)
384:               if mod(n, 2) {
385:                 let zx := mul(z, x)
386:                 if and(iszero(iszero(x)), iszero(eq(div(zx, x), z))) {
387:                   revert(0, 0)
388:                 }
389:                 let zxRound := add(zx, half)
390:                 if lt(zxRound, zx) {
391:                   revert(0, 0)
392:                 }
393:                 z := div(zxRound, base)
394:               }
395:             }
396:           }
397:         }

350-397

<a name="N-03"></a>[N-03] Do not calculate constants

Due to how constant variables are implemented (replacements at compile-time), an expression assigned to a constant variable is recomputed each time that the variable is used, which wastes some gas.

Instances (9):

<details> <summary>see instances</summary>

File: contracts/rwaOracles/RWADynamicOracle.sol

27:   bytes32 public constant SETTER_ROLE = keccak256("SETTER_ROLE");

28:   bytes32 public constant PAUSER_ROLE = keccak256("PAUSER_ROLE");

343:   uint256 private constant ONE = 10 ** 27;

27, 28, 343

File: contracts/usdy/rUSDY.sol

97:   bytes32 public constant USDY_MANAGER_ROLE = keccak256("ADMIN_ROLE");

98:   bytes32 public constant MINTER_ROLE = keccak256("MINTER_ROLE");

99:   bytes32 public constant PAUSER_ROLE = keccak256("PAUSER_ROLE");

100:   bytes32 public constant BURNER_ROLE = keccak256("BURN_ROLE");

101:   bytes32 public constant LIST_CONFIGURER_ROLE =
102:         keccak256("LIST_CONFIGURER_ROLE");

97, 98, 99, 100, 101-102

File: contracts/usdy/rUSDYFactory.sol

44:   bytes32 public constant DEFAULT_ADMIN_ROLE = bytes32(0);

44

</details>

<a name="N-04"></a>[N-04] Function can be declared as pure

Functions below do not change the state of the contract and can be declared as pure. It will save gas and make the code more readable.

Instances (1):

File: contracts/usdy/rUSDY.sol

626:   function _beforeTokenTransfer(
627:         address from,
628:         address to,
629:         uint256
630:       ) internal view {
631:         // Check constraints when `transferFrom` is called to facliitate
632:         // a transfer between two parties that are not `from` or `to`.
633:         if (from != msg.sender && to != msg.sender) {
634:           require(!_isBlocked(msg.sender), "rUSDY: 'sender' address blocked");
635:           require(!_isSanctioned(msg.sender), "rUSDY: 'sender' address sanctioned");
636:           require(
637:             _isAllowed(msg.sender),
638:             "rUSDY: 'sender' address not on allowlist"
639:           );
640:         }
641:     
642:         if (from != address(0)) {
643:           // If not minting
644:           require(!_isBlocked(from), "rUSDY: 'from' address blocked");
645:           require(!_isSanctioned(from), "rUSDY: 'from' address sanctioned");
646:           require(_isAllowed(from), "rUSDY: 'from' address not on allowlist");
647:         }
648:     
649:         if (to != address(0)) {
650:           // If not burning
651:           require(!_isBlocked(to), "rUSDY: 'to' address blocked");
652:           require(!_isSanctioned(to), "rUSDY: 'to' address sanctioned");
653:           require(_isAllowed(to), "rUSDY: 'to' address not on allowlist");
654:         }
655:       }

626-655

<a name="N-05"></a>[N-05] Named imports of parent contracts are missing

Instances (20):

<details> <summary>see instances</summary>

File: contracts/bridge/DestinationBridge.sol

28:   AxelarExecutable,

29:   MintTimeBasedRateLimiter,

30:   Ownable,

31:   Pausable

28, 29, 30, 31

File: contracts/bridge/SourceBridge.sol

11: contract SourceBridge is Ownable, Pausable, IMulticall {

11: contract SourceBridge is Ownable, Pausable, IMulticall {

11: contract SourceBridge is Ownable, Pausable, IMulticall {

11, 11, 11

File: contracts/rwaOracles/RWADynamicOracle.sol

22: contract RWADynamicOracle is IRWAOracle, AccessControlEnumerable, Pausable {

22: contract RWADynamicOracle is IRWAOracle, AccessControlEnumerable, Pausable {

22: contract RWADynamicOracle is IRWAOracle, AccessControlEnumerable, Pausable {

22, 22, 22

File: contracts/usdy/rUSDY.sol

58:   Initializable,

59:   ContextUpgradeable,

60:   PausableUpgradeable,

61:   AccessControlEnumerableUpgradeable,

62:   BlocklistClientUpgradeable,

63:   AllowlistClientUpgradeable,

64:   SanctionsListClientUpgradeable,

65:   IERC20Upgradeable,

66:   IERC20MetadataUpgradeable

58, 59, 60, 61, 62, 63, 64, 65, 66

File: contracts/usdy/rUSDYFactory.sol

43: contract rUSDYFactory is IMulticall {

43

</details>

<a name="N-06"></a>[N-06] Not using the named return variables anywhere in the function is confusing

Consider changing the variable to be an unnamed one, since the variable is never assigned, nor is it returned by name

Instances (4):

File: contracts/rwaOracles/RWADynamicOracle.sol

75:   function getPrice() public view whenNotPaused returns (uint256 price) {

104:   function simulateRange(
105:         uint256 blockTimeStamp,
106:         uint256 dailyIR,
107:         uint256 endTime,
108:         uint256 startTime,
109:         uint256 rangeStartPrice
110:       ) external view returns (uint256 price) {

262:   function derivePrice(
263:         Range memory currentRange,
264:         uint256 currentTime
265:       ) internal pure returns (uint256 price) {

345:   function _rpow(
346:         uint256 x,
347:         uint256 n,
348:         uint256 base
349:       ) internal pure returns (uint256 z) {

75, 104-110, 262-265, 345-349

<a name="N-07"></a>[N-07] Strings should use double quotes rather than single quotes

See the Solidity Style Guide

Instances (9):

<details> <summary>see instances</summary>

File: contracts/usdy/rUSDY.sol

634:       require(!_isBlocked(msg.sender), "rUSDY: 'sender' address blocked");

635:       require(!_isSanctioned(msg.sender), "rUSDY: 'sender' address sanctioned");

638:         "rUSDY: 'sender' address not on allowlist"

644:       require(!_isBlocked(from), "rUSDY: 'from' address blocked");

645:       require(!_isSanctioned(from), "rUSDY: 'from' address sanctioned");

646:       require(_isAllowed(from), "rUSDY: 'from' address not on allowlist");

651:       require(!_isBlocked(to), "rUSDY: 'to' address blocked");

652:       require(!_isSanctioned(to), "rUSDY: 'to' address sanctioned");

653:       require(_isAllowed(to), "rUSDY: 'to' address not on allowlist");

634, 635, 638, 644, 645, 646, 651, 652, 653

</details>

<a name="N-08"></a>[N-08] Some contract names don't follow the Solidity naming conventions

See the contract-and-library-names section of the Solidity Style Guide

Instances (2):

File: contracts/usdy/rUSDY.sol

57: contract rUSDY is

57

File: contracts/usdy/rUSDYFactory.sol

43: contract rUSDYFactory is IMulticall {

43

<a name="N-09"></a>[N-09] Unused structs

If these serve no purpose, they should be safely removed

Instances (2):

File: contracts/bridge/DestinationBridge.sol

374:   struct TxnThreshold {
375:         uint256 numberOfApprovalsNeeded;
376:         address[] approvers;
377:       }

379:   struct Transaction {
380:         address sender;
381:         uint256 amount;
382:       }

374-377, 379-382

<a name="N-10"></a>[N-10] Functions not used internally could be marked external

Instances (14):

<details> <summary>see instances</summary>

File: contracts/usdy/rUSDY.sol

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

202:   function symbol() public pure returns (string memory) {

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

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

226:   function balanceOf(address _account) public view returns (uint256) {

245:   function transfer(address _recipient, uint256 _amount) public returns (bool) {

256:   function allowance(

276:   function approve(address _spender, uint256 _amount) public returns (bool) {

301:   function transferFrom(

327:   function increaseAllowance(

353:   function decreaseAllowance(

372:   function getTotalShares() public view returns (uint256) {

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

416:   function transferShares(

194, 202, 209, 216, 226, 245, 256, 276, 301, 327, 353, 372, 381, 416

</details>

<a name="N-11"></a>[N-11] OpenZeppelin libraries should be upgraded to a newer version

These contracts import some OpenZeppelin libraries, but they are using an old version.

Instances (13):

<details> <summary>see instances</summary>

File: contracts/bridge/DestinationBridge.sol

23: import "contracts/external/openzeppelin/contracts/access/Ownable.sol";

24: import "contracts/external/openzeppelin/contracts/security/Pausable.sol";

23, 24

File: contracts/bridge/SourceBridge.sol

8: import "contracts/external/openzeppelin/contracts/access/Ownable.sol";

9: import "contracts/external/openzeppelin/contracts/security/Pausable.sol";

8, 9

File: contracts/rwaOracles/RWADynamicOracle.sol

19: import "contracts/external/openzeppelin/contracts/access/AccessControlEnumerable.sol";

20: import "contracts/external/openzeppelin/contracts/security/Pausable.sol";

19, 20

File: contracts/usdy/rUSDY.sol

18: import "contracts/external/openzeppelin/contracts-upgradeable/token/ERC20/IERC20Upgradeable.sol";

19: import "contracts/external/openzeppelin/contracts-upgradeable/token/ERC20/IERC20MetadataUpgradeable.sol";

20: import "contracts/external/openzeppelin/contracts-upgradeable/proxy/Initializable.sol";

21: import "contracts/external/openzeppelin/contracts-upgradeable/utils/ContextUpgradeable.sol";

22: import "contracts/external/openzeppelin/contracts-upgradeable/security/PausableUpgradeable.sol";

23: import "contracts/external/openzeppelin/contracts-upgradeable/access/AccessControlEnumerableUpgradeable.sol";

18, 19, 20, 21, 22, 23

File: contracts/usdy/rUSDYFactory.sol

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

19

</details>

<a name="N-12"></a>[N-12] Non-usage of specific imports

The current form of relative path import is not recommended for use because it can unpredictably pollute the namespace. Instead, the Solidity docs recommend specifying imported symbols explicitly. https://docs.soliditylang.org/en/v0.8.15/layout-of-source-files.html#importing-other-source-files

Instances (32):

<details> <summary>see instances</summary>

File: contracts/bridge/DestinationBridge.sol

18: import "contracts/interfaces/IAxelarGateway.sol";

19: import "contracts/interfaces/IAxelarGasService.sol";

20: import "contracts/external/axelar/AxelarExecutable.sol";

21: import "contracts/interfaces/IRWALike.sol";

22: import "contracts/interfaces/IAllowlist.sol";

23: import "contracts/external/openzeppelin/contracts/access/Ownable.sol";

24: import "contracts/external/openzeppelin/contracts/security/Pausable.sol";

25: import "contracts/bridge/MintRateLimiter.sol";

18, 19, 20, 21, 22, 23, 24, 25

File: contracts/bridge/SourceBridge.sol

3: import "contracts/interfaces/IAxelarGateway.sol";

4: import "contracts/interfaces/IAxelarGasService.sol";

5: import "contracts/interfaces/IMulticall.sol";

6: import "contracts/interfaces/IRWALike.sol";

8: import "contracts/external/openzeppelin/contracts/access/Ownable.sol";

9: import "contracts/external/openzeppelin/contracts/security/Pausable.sol";

3, 4, 5, 6, 8, 9

File: contracts/rwaOracles/RWADynamicOracle.sol

18: import "contracts/rwaOracles/IRWAOracle.sol";

19: import "contracts/external/openzeppelin/contracts/access/AccessControlEnumerable.sol";

20: import "contracts/external/openzeppelin/contracts/security/Pausable.sol";

18, 19, 20

File: contracts/usdy/rUSDY.sol

18: import "contracts/external/openzeppelin/contracts-upgradeable/token/ERC20/IERC20Upgradeable.sol";

19: import "contracts/external/openzeppelin/contracts-upgradeable/token/ERC20/IERC20MetadataUpgradeable.sol";

20: import "contracts/external/openzeppelin/contracts-upgradeable/proxy/Initializable.sol";

21: import "contracts/external/openzeppelin/contracts-upgradeable/utils/ContextUpgradeable.sol";

22: import "contracts/external/openzeppelin/contracts-upgradeable/security/PausableUpgradeable.sol";

23: import "contracts/external/openzeppelin/contracts-upgradeable/access/AccessControlEnumerableUpgradeable.sol";

24: import "contracts/usdy/blocklist/BlocklistClientUpgradeable.sol";

25: import "contracts/usdy/allowlist/AllowlistClientUpgradeable.sol";

26: import "contracts/sanctions/SanctionsListClientUpgradeable.sol";

27: import "contracts/interfaces/IUSDY.sol";

28: import "contracts/rwaOracles/IRWADynamicOracle.sol";

18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28

File: contracts/usdy/rUSDYFactory.sol

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

20: import "contracts/Proxy.sol";

21: import "contracts/usdy/rUSDY.sol";

22: import "contracts/interfaces/IMulticall.sol";

19, 20, 21, 22

</details>