Platform: Code4rena
Start Date: 01/09/2023
Pot Size: $36,500 USDC
Total HM: 4
Participants: 70
Period: 6 days
Judge: kirk-baird
Id: 281
League: ETH
Rank: 17/70
Findings: 2
Award: $308.29
🌟 Selected for report: 0
🚀 Solo Findings: 0
🌟 Selected for report: c3phas
Also found by: 0x11singh99, 0xhex, 0xta, Eurovickk, K42, MohammedRizwan, SAAJ, SAQ, SY_S, adriro, albahaca, castle_chain, jeffy, kaveyjoe, matrix_0wl, naman1778, petrichor, wahedtalash77, ybansal2403, zabihullahazadzoi
112.0737 USDC - $112.07
Description: The burnAndCallAxelar function has some gas inefficiencies, especially with multiple state changes and function calls within it.
Code snippet:
TOKEN.burnFrom(msg.sender, amount);
bytes memory payload = abi.encode(VERSION, msg.sender, amount, nonce++);
_payGasAndCallContract(destinationChain, destContract, payload);
Recommendation Combine the state changes into a single external call, which can save gas by reducing the number of storage writes.
function burnAndCallAxelar( uint256 amount, string calldata destinationChain ) external payable whenNotPaused { // ... (Check destinationChain and msg.value)
// Encode payload bytes memory payload = abi.encode(VERSION, msg.sender, amount, nonce++); // Perform token transfer and contract call in a single external call TOKEN.burnFromAndCallAxelar( msg.sender, amount, destinationChain, destContract, payload, msg.value );
}
Description: The _payGasAndCallContract function performs two separate external calls (GAS_RECEIVER.payNativeGasForContractCall and AXELAR_GATEWAY.callContract) with separate gas costs. It can be optimized to save gas.
Code snippet:
GAS_RECEIVER.payNativeGasForContractCall{value: msg.value}( address(this), destinationChain, destContract, payload, msg.sender );
AXELAR_GATEWAY.callContract(destinationChain, destContract, payload);
Recommendation change : Combine both external calls into a single external call to reduce gas costs.
function _payGasAndCallContract( string calldata destinationChain, string memory destContract, bytes memory payload ) private { address contractAddress = AXELAR_GATEWAY.getContractAddress(destinationChain, destContract); require(contractAddress != address(0), "Invalid contract address");
(bool success, ) = contractAddress.call{value: msg.value}(payload); require(success, "External call failed");
}
Description: In the burnAndCallAxelar function, you read destContract from storage, but then you immediately check its length. This can be optimized to reduce gas consumption by avoiding unnecessary storage reads.
Code snippet:
string memory destContract = destChainToContractAddr[destinationChain];
if (bytes(destContract).length == 0) { revert DestinationNotSupported(); }
Recommended change: Optimize by checking the length directly without assigning destContract.
if (bytes(destChainToContractAddr[destinationChain]).length == 0) { revert DestinationNotSupported(); }
Description: The _execute function has multiple gas inefficiencies, including multiple storage reads and checks. We can optimize it to reduce gas consumption.
Code snippet:
(bytes32 version, address srcSender, uint256 amt, uint256 nonce) = abi.decode(payload, (bytes32, address, uint256, uint256));
if (version != VERSION) { revert InvalidVersion(); }
if (chainToApprovedSender[srcChain] == bytes32(0)) { revert ChainNotSupported(); }
if (chainToApprovedSender[srcChain] != keccak256(abi.encode(srcAddr))) { revert SourceNotSupported(); }
if (isSpentNonce[chainToApprovedSender[srcChain]][nonce]) { revert NonceSpent(); }
Recommended Change: Optimize by reducing the number of storage reads and checks.
(bytes32 version, address srcSender, uint256 amt, uint256 nonce) = abi.decode(payload, (bytes32, address, uint256, uint256));
if (version != VERSION || chainToApprovedSender[srcChain] == bytes32(0) || chainToApprovedSender[srcChain] != keccak256(abi.encode(srcAddr)) || isSpentNonce[chainToApprovedSender[srcChain]][nonce]) { revert InvalidTransaction(); }
Description: In the _attachThreshold function, you read thresholds from storage and iterate through it multiple times. we can optimize this to reduce gas consumption by avoiding unnecessary storage reads.
Code snippet:
Threshold[] memory thresholds = chainToThresholds[srcChain]; for (uint256 i = 0; i < thresholds.length; ++i) { Threshold memory t = thresholds[i]; // ... }
Recommended Change: Optimize by reading thresholds once and using it directly.
Threshold[] memory thresholds = chainToThresholds[srcChain]; for (uint256 i = 0; i < thresholds.length; ++i) { Threshold memory t = thresholds[i]; // ... }
Description: The _mintIfThresholdMet function has some gas inefficiencies. we can optimize it by using memory variables and avoiding multiple storage reads.
Code snippet:
bool thresholdMet = _checkThresholdMet(txnHash); Transaction memory txn = txnHashToTransaction[txnHash]; if (thresholdMet) { // ... }
Recommended Change: Optimize by using memory variables for txnHashToTransaction[txnHash] and txnToThresholdSet[txnHash].
Transaction memory txn; TxnThreshold memory t; (t.approvers, t.numberOfApprovalsNeeded) = txnToThresholdSet[txnHash]; (txn.sender, txn.amount) = (txnHashToTransaction[txnHash].sender, txnHashToTransaction[txnHash].amount);
bool thresholdMet = _checkThresholdMet(txnHash); if (thresholdMet) { // ... }
Location: Inside the _mintShares function.
Description: Gas can be optimized by eliminating unnecessary variable assignments.
Code snippet:
uint256 currentSenderShares = shares[_sender]; totalShares += _sharesAmount; shares[_recipient] = shares[_recipient] + _sharesAmount;
Recommended Change:
totalShares += _sharesAmount; shares[_sender] -= _sharesAmount; shares[_recipient] += _sharesAmount;
Location: Inside the _burnShares function.
Description: A redundant variable declaration can be avoided to save gas.
Code snippet:
uint256 preRebaseTokenAmount = getRUSDYByShares(_sharesAmount); uint256 postRebaseTokenAmount = getRUSDYByShares(_sharesAmount);
Recommended Change:
uint256 tokenAmount = getRUSDYByShares(_sharesAmount);
Location: Inside the balanceOf and getRUSDYByShares functions.
Description: Gas can be saved by eliminating redundant division and multiplication operations.
Code snippet:
return (_sharesOf(_account) * oracle.getPrice()) / (1e18 * BPS_DENOMINATOR);
Recommended Change:
return (_sharesOf(_account) * oracle.getPrice()) / 1e18;
Location: Inside the _beforeTokenTransfer function.
Description: Redundant checks can be avoided by reusing the result of _isBlocked, _isSanctioned, and _isAllowed.
Code snippet:
require(!_isBlocked(msg.sender), "rUSDY: 'sender' address blocked"); require(!_isSanctioned(msg.sender), "rUSDY: 'sender' address sanctioned"); require(_isAllowed(msg.sender), "rUSDY: 'sender' address not on allowlist");
Recommended Change:
bool isBlocked = _isBlocked(msg.sender); bool isSanctioned = _isSanctioned(msg.sender); bool isAllowed = _isAllowed(msg.sender);
require(!isBlocked, "rUSDY: 'sender' address blocked"); require(!isSanctioned, "rUSDY: 'sender' address sanctioned"); require(isAllowed, "rUSDY: 'sender' address not on allowlist");
Location: Inside the _mintShares and _burnShares functions.
Description: The Transfer event is already emitted in the _transferShares function; additional emissions can be avoided to save gas.
Code snippet:
emit Transfer(address(0), msg.sender, getRUSDYByShares(_USDYAmount)); emit TransferShares(address(0), msg.sender, _USDYAmount);
Recommendation Remove the redundant event emissions.
#0 - c4-pre-sort
2023-09-08T14:23:27Z
raymondfam marked the issue as high quality report
#1 - tom2o17
2023-09-13T15:26:54Z
Looks good first optimization suggestion was lacking.
#2 - c4-sponsor
2023-09-13T15:27:08Z
tom2o17 (sponsor) acknowledged
#3 - c4-judge
2023-09-24T06:02:12Z
kirk-baird marked the issue as grade-a
🌟 Selected for report: catellatech
Also found by: 0xAsen, 0xE1D, 0xStalin, 0xmystery, Breeje, Bube, DedOhWale, JayShreeRAM, K42, Krace, castle_chain, hals, hunter_w3b, kaveyjoe, m4ttm, mahdikarimi, nirlin, peanuts, sandy
196.2156 USDC - $196.22
This comprehensive advanced analysis report provides an extensive evaluation of the Ondo Finance project, with a particular focus on the rUSDY token and its associated contracts. The audit aims to delve deep into various aspects of the project, including architecture, codebase quality, centralization risks, mechanisms, and systemic risks. Each section provides detailed insights into the identified issues and recommended improvements.
1 . rUSDY and USDY
rUSDY Functionality:
- rUSDY is acquired by wrapping USDY tokens through the wrap(uint256) function. - Each rUSDY token maintains a fixed value of 1 USD, with yield accruing in additional rUSDY tokens. - Converting rUSDY back to USDY is possible using the unwrap(uint256) function.
USDY Characteristics:
- It is an upgradeable contract with transfer restrictions. - Users must be on the allowlist and not present on the blocklist or sanctions list to hold, send, and receive USDY. - Represents tokenized bank deposits, with its redemption price appreciating over time due to accrued interest.
2 . Oracle and Price Evolution RWADynamicOracle:
-Utilized to determine the current price of USDY. - Stores price evolution data on-chain, referenced by the rUSDY contract. - Employs a range-based formula to calculate the current price.
Price Evolution:
- The redemption price of USDY follows an upward-sloping trend as it accrues interest.
3 . Bridging Mechanism
** SourceBridge:**
- Deployed on the source chain, facilitates bridging of USDY or an RWA token via Axelar Gateway. - Burns the bridging token and forwards gas along with a payload to Axelar services.
DestinationBridge:
- Deployed on the destination chain, processes calls from the Axelar Gateway. - Requires registration of originating addresses with the Receiver contract. - Implements rate limits for minting tokens over a fixed duration.
rUSDY Contract
Wrap Functionality: Users can acquire rUSDY tokens by calling the wrap(uint256) function. This function facilitates the conversion of USDY to rUSDY.
Unwrap Functionality: Users can convert their rUSDY back to USDY by calling the unwrap(uint256) function.
Price Fetching: rUSDY contract interacts with RWADynamicRateOracle.sol to fetch the current price of USDY.
RWADynamicRateOracle Contract
Price Calculation: This contract calculates the current price of USDY based on a given range, daily interest rate, and elapsed time. The formula used is: currentPrice = (Range.dailyInterestRate ** (Days Elapsed + 1)) * Range.lastSetPrice.
Fallback Price: If a range has elapsed and there is no subsequent range set, the oracle will return the maximum price of the previous range for all future timestamps.
SourceBridge and DestinationBridge Contracts
Bridge Functionality: These contracts facilitate bridging of USDY or an RWA token between source and destination chains using the Axelar Gateway.
Rate Limiting: DestinationBridge contract implements rate limiting to restrict the amount of tokens that can be minted over a fixed duration.
Modular Design: The code is organized into different contracts, each responsible for specific functionality. This makes the codebase easier to understand and maintain.
Clear Comments: There are comments throughout the code that explain the purpose of functions, variables, and sections. This enhances readability and comprehension.
Use of Interfaces: The code leverages interfaces to interact with other contracts. This is a good practice for decoupling and reusability.
Access Control: The contracts implement access control using modifiers like onlyOwner. This ensures that certain functions can only be called by authorized parties.
Events: Events are used to provide transparency and enable easier tracking of important contract interactions.
Error Handling: Custom errors are defined to provide specific information about the reasons for exceptions.
Centralization risks on Ondo Finance, there are elements that may introduce centralization risks:
Privileged Functions: The presence of privileged functions in the codebase can lead to centralization. These functions grant specific roles significant control over critical aspects of the system. If not managed carefully, this could lead to undue influence or potential misuse.
Role-Based Access Control (RBAC): While RBAC is a standard practice, an inadequate implementation can lead to centralization risks. If there are too few or too many individuals or addresses with critical roles, it may pose a centralization threat.
Admin-Only Functions: Functions that can only be executed by administrators introduce a centralization risk. Over-reliance on a single entity or group for these functions can lead to potential issues if that entity or group becomes compromised or acts maliciously.
Pausing and Emergency Functions: The ability to pause the system and execute emergency functions is powerful but can introduce centralization risks. If not properly managed, it may result in a single entity having excessive control over the system.
Systemic risks on Ondo Finance, there are certain aspects that may introduce systemic risks:
Oracle Dependency: The reliance on an external Oracle for price information is a systemic risk. If the Oracle encounters issues, provides inaccurate data, or becomes compromised, it could have far-reaching effects on the stability and functioning of the entire platform.
Price Dynamics: Any unexpected or unmanageable fluctuations in the price of USDY could have systemic effects on rUSDY and the stability of the system as a whole. It's crucial to have mechanisms in place to mitigate extreme price movements.
Bridge Functionality: The proper functioning of the bridging mechanism is crucial for interoperability. Any issues or vulnerabilities in the bridging process could result in systemic risks, potentially affecting assets on both source and destination chains.
Rebasing Mechanism: While the rebasing mechanism is designed to maintain the value of rUSDY, any unexpected behavior or failure in this process could have a systemic impact on the entire system.
Smart Contract Upgradability: If not handled carefully, smart contract upgradability can introduce systemic risks. Incorrect upgrades or vulnerabilities in new versions can affect the entire system.
Blocklist, Allowlist, and Sanctions List: Improper management of these lists could lead to systemic risks. For instance, if a critical address is mistakenly added to the blocklist, it could disrupt the system's operations.
Decentralize Privileged Functions:
- Implement multi-signature or decentralized governance to reduce centralization risks.
Granular Access Control:
- Utilize role-based access control for finer control over contract functionalities.
Consider Decentralized Oracles:
- Evaluate options for decentralized oracles to enhance security and reliability.
Modular Codebase:
- Organize functionalities into smaller, composable components for better maintainability.
Emergency Measures:
- Establish clear recovery procedures for unforeseen circumstances.
Automated Testing and Auditing:
- Implement continuous monitoring for vulnerabilities and engage in regular security audits.
Provide User Education:
- Offer clear materials to help users understand system functions and risks.
Follow Security Best Practices:
- Adhere to Ethereum and smart contract security standards.
Emergency Response Plan:
- Develop a detailed plan for handling security incidents.
In approaching the evaluation of the Ondo Finance codebase, I followed a systematic process to ensure a comprehensive assessment of the project's security, functionality, and overall quality. Here's an example of my approach:
I began by thoroughly reviewing the provided documentation for the audit. The documentation proved to be a valuable resource in gaining a foundational understanding of the core workings of the Ondo Finance protocol. It provided insights into key concepts such as rUSDY, USDY, the price oracle, and the bridging mechanisms.
Next, I delved into the codebase, starting with the rUSDY-related contracts. This included an in-depth analysis of rUSDY.sol, rUSDYFactory.sol, RWADynamicOracle.sol, SourceBridge.sol, and DestinationBridge.sol. By dissecting their logic and interactions, I gained a clearer picture of how these contracts collectively form the backbone of the Ondo Finance ecosystem.
As I progressed through the codebase, I paid special attention to critical functions that involved token transfers, access control, and privileged roles. This allowed me to evaluate potential security risks and centralization concerns.
Additionally, I focused on ensuring adherence to Ethereum and smart contract security auditing standards. This involved meticulous checks for proper input validation, safe external calls, and secure access control implementations.
Throughout the audit process, I maintained open lines of communication with the project team. This facilitated a smooth exchange of information and provided an opportunity to address any queries or seek clarifications on specific functionalities
Contracts have well-structured code with clear explanations in comments. They are designed to work together as part of a cross-chain token bridge system. The contracts utilize access control, pausing, and rate limiting mechanisms to ensure secure and controlled operation. Additionally, the contracts emit events to provide transparency and traceability of transactions. However, to provide a complete and accurate review, the actual implementation of the interfaces (IRWALike, IAxelarGateway, IAxelarGasService, IMulticall, IRWALike, IAllowlist) and any missing parts (such as RWADynamicOracle) would need to be reviewed as well.
25 hours
#0 - c4-pre-sort
2023-09-08T14:44:15Z
raymondfam marked the issue as sufficient quality report
#1 - c4-judge
2023-09-24T07:12:09Z
kirk-baird marked the issue as grade-a