Platform: Code4rena
Start Date: 19/01/2024
Pot Size: $36,500 USDC
Total HM: 9
Participants: 113
Period: 3 days
Judge: 0xsomeone
Id: 322
League: ETH
Rank: 44/113
Findings: 2
Award: $62.25
🌟 Selected for report: 0
🚀 Solo Findings: 0
🌟 Selected for report: c3phas
Also found by: 0x11singh99, Raihan, dharma09, hunter_w3b, slvDev
38.8402 USDC - $38.84
The bot did an amazing job on this audit,but still missed some findings
NOTE: This instance was missed by the bot. Use immutable if you want to assign a permanent value at construction. Use constants if you already know the permanent value. Both get directly embedded in bytecode, saving SLOAD.
2.1K
4
2.1 * 4 = 8400 Gas
File: src/bridge_adapters/DecentBridgeAdapter.sol 19 address bridgeToken;
- address bridgeToken; + address immutable bridgeToken;
File: src/DecentEthRouter.sol 15 IDecentBridgeExecutor public executor;
File: src/DecentEthRouter.sol - IDecentBridgeExecutor public executor; + IDecentBridgeExecutor public immutable executor;
File: src/DecentBridgeExecutor.sol 9 IWETH weth; 10 bool public gasCurrencyIsEth; // for chains that use ETH as gas currency
- IWETH weth; + IWETH immutable weth; - bool public gasCurrencyIsEth; + bool public immutable gasCurrencyIsEth;
This instance was missed by the bot.
97
16
97 * 16 = 1552 Gas
File: main/src/UTB.sol 76 wrapped.deposit{value: swapParams.amountIn}(); 77 swapParams.tokenIn = address(wrapped); 98 wrapped.withdraw(amountOut); 143 executor.execute{value: amountOut}( 152 IERC20(tokenOut).approve(address(executor), amountOut); 153 executor.execute( // @audit cache feeCollector in local variable 233 if (address(feeCollector) != address(0)) { 242 address(feeCollector), 248 feeCollector.collectFees{value: value}(fees, packedInfo, signature);
https://github.com/code-423n4/2024-01-decent/blob/main/src/UTB.sol#L76
File: src/DecentBridgeExecutor.sol // @audit cache weth in local variable 30 uint256 balanceBefore = weth.balanceOf(address(this)); 41 (balanceBefore - weth.balanceOf(address(this))); 44 weth.transfer(from, remainingAfterCall);
File: src/DecentEthRouter.sol 266 if (weth.balanceOf(address(this)) < _amount) { 273 weth.transfer(_to, _amount); 275 weth.withdraw(_amount); 279 weth.approve(address(executor), _amount);
Using named arguments for struct means that the compiler needs to organize the fields in memory before doing the assignment, which wastes gas. Set each field directly in storage (use dot-notation), or use the unnamed version of the constructor.
File: src/DecentEthRouter.sol 170 ICommonOFT.LzCallParams memory callParams = ICommonOFT.LzCallParams({ 171 refundAddress: payable(msg.sender), 172 zroPaymentAddress: address(0x0), 173 adapterParams: adapterParams 174 });
File: src/swappers/UniSwapper. 130 .ExactInputParams({ 131 path: swapParams.path, 132 recipient: address(this), 133 amountIn: swapParams.amountIn, 134 amountOutMinimum: swapParams.amountOut 135 }); 150 .ExactOutputParams({ 151 path: swapParams.path, 152 recipient: address(this), 153 //deadline: block.timestamp, 154 amountOut: swapParams.amountOut, 155 amountInMaximum: swapParams.amountIn 156 });
https://github.com/code-423n4/2024-01-decent/blob/main/src/swappers/UniSwapper.sol#L130-L135
When fetching data from a storage location, assigning the data to a memory variable causes all fields of the struct/array to be read from storage, which incurs a Gcoldsload (2100 gas) for each field of the struct/array.
If the fields are read from the new memory variable, they incur an additional MLOAD rather than a cheap stack read.
Instead of declearing the variable with the memory keyword, declaring the variable with the storage keyword and caching any fields that need to be re-read in stack variables, will be much cheaper, only incuring the Gcoldsload for the fields actually read.
The only time it makes sense to read the whole struct/array into a memory variable, is if the full struct/array is being returned by the function, is being passed to a function that requires memory, or if the array/struct is being read from another memory array/struct
File: src/bridge_adapters/DecentBridgeAdapter.sol 51 SwapParams memory swapParams = abi.decode( 103 SwapParams memory swapParams = abi.decode( 134 SwapParams memory swapParams = abi.decode(
File: src/DecentEthRouter.sol 170 ICommonOFT.LzCallParams memory callParams = ICommonOFT.LzCallParams({
File: src/bridge_adapters/StargateBridgeAdapter.sol 202 SwapParams memory swapParams = abi.decode(
File: src/swappers/UniSwapper.sol 129 IV3SwapRouter.ExactInputParams memory params = IV3SwapRouter 149 IV3SwapRouter.ExactOutputParams memory params = IV3SwapRouter
https://github.com/code-423n4/2024-01-decent/blob/main/src/swappers/UniSwapper.sol#L129
File: src/UTB.sol 69 SwapParams memory swapParams = abi.decode( 181 SwapParams memory newPostSwapParams = abi.decode(
https://github.com/code-423n4/2024-01-decent/blob/main/src/UTB.sol#L69
NOTE: This instance was missed by the bot.
When a function with a memory array is called externally, the abi.decode () step has to use a for-loop to copy each index of the calldata to the memory index. Each iteration of this for-loop costs at least 60 gas (i.e. 60 * <mem_array>.length)
. Using calldata directly, obliviates the need for such a loop in the contract code and runtime execution.
File: src/DecentBridgeExecutor.sol 73 bytes memory callPayload
File: src/DecentEthRouter.sol 120 bytes memory payload 203 bytes memory additionalPayload 243 bytes memory _payload
File: src/bridge_adapters/StargateBridgeAdapter.sol 71 SwapInstructions memory postBridge, 75 bytes memory payload, 189 bytes memory payload
File: src/swappers/UniSwapper.sol 33 SwapParams memory newSwapParams, 34 bytes memory payload 59 bytes memory swapPayload
https://github.com/code-423n4/2024-01-decent/blob/main/src/swappers/UniSwapper.sol#L33
File: src/UTB.sol 312 SwapInstructions memory postBridge, 315 bytes memory payload,
https://github.com/code-423n4/2024-01-decent/blob/main/src/UTB.sol#L312
File: src/UTBExecutor.sol 22 bytes memory payload, 44 bytes memory payload,
https://github.com/code-423n4/2024-01-decent/blob/main/src/UTBExecutor.sol#L22
File: src/UTBFeeCollector.sol 46 bytes memory packedInfo, 47 bytes memory signature
https://github.com/code-423n4/2024-01-decent/blob/main/src/UTBFeeCollector.sol#L46
File: src/bridge_adapters/DecentBridgeAdapter.sol 30 function getId() public pure returns (uint8) { 31 return BRIDGE_ID; 32 }
File: src/bridge_adapters/StargateBridgeAdapter.sol 41 function getId() public pure returns (uint8) { 42 return BRIDGE_ID; 43 }
File: src/swappers/UniSwapper.sol 28 function getId() public pure returns (uint8) { 29 return SWAPPER_ID; 30 }
https://github.com/code-423n4/2024-01-decent/blob/main/src/swappers/UniSwapper.sol#L28-L30
A public storage variable has an implicit public function of the same name. A public function increases the size of the jump table and adds bytecode to read the variable in question. That makes the contract larger.
File:/home/x0w3r/progress-ON/Decent/scope/BaseAdapter.sol 7 address public bridgeExecutor;
https://github.com/code-423n4/2024-01-decent/blob/main/src/bridge_adapters/BaseAdapter.sol#L7
File: /home/x0w3r/progress-ON/Decent/scope/DcntEth.sol 6 address public router;
File: /home/x0w3r/progress-ON/Decent/scope/DecentBridgeAdapter.sol 15 IDecentEthRouter public router;
File: /home/x0w3r/progress-ON/Decent/scope/DecentBridgeExecutor.sol 10 bool public gasCurrencyIsEth; // for chains that use ETH as gas currency
File: /home/x0w3r/progress-ON/Decent/scope/DecentEthRouter.sol 13 IWETH public weth; 14 IDcntEth public dcntEth; 15 IDecentBridgeExecutor public executor; 22 bool public gasCurrencyIsEth; // for chains that use ETH as gas currency
File:/home/x0w3r/progress-ON/Decent/scope/StargateBridgeAdapter.sol 24 address public stargateEth;
File: /home/x0w3r/progress-ON/Decent/scope/UniSwapper.sol 17 address public uniswap_router; 18 address payable public wrapped;
https://github.com/code-423n4/2024-01-decent/blob/main/src/swappers/UniSwapper.sol#L17
2100
2
2 * 2100 = 4200 Gas
File: src/DecentBridgeExecutor.sol 77 if (!gasCurrencyIsEth || !deliverEth) {
The if statement has two checks !gasCurrencyIsEth
and !deliverEth
where the first check involves making a state read while the second check only compares a variable to a function parameter.
According to the rules of short circuit, if the first check is true, we do not have to do the second check thus in this case, we should make sure the first check is the cheapest to do.
By reordering as shown below, we can avoid making the state read if !gasCurrencyIsEth which would save us ~2100
Gas.
- if (!gasCurrencyIsEth || !deliverEth) { + if (!deliverEth || !gasCurrencyIsEth ) {
File: src/DecentEthRouter.sol 272 if (!gasCurrencyIsEth || !deliverEth) {
If data can fit into 32 bytes, then you should use bytes32 datatype rather than bytes or strings as it is cheaper in solidity.
File: main/src/UTBFeeCollector.sol 10 string constant BANNER = "\x19Ethereum Signed Message:\n32";
https://github.com/code-423n4/2024-01-decent/blob/main/src/UTBFeeCollector.sol#L10
When reverting in solidity code, it is common practice to use a require or revert statement to revert execution with an error message. This can in most cases be further optimized by using assembly to revert with the error message.
File: src/bridge_adapters/BaseAdapter.sol 12 require(
https://github.com/code-423n4/2024-01-decent/blob/main/src/bridge_adapters/BaseAdapter.sol#L12
File: src/DcntEth.sol 9 require(msg.sender == router);
File: src/bridge_adapters/DecentBridgeAdapter.sol 91 require(
File: src/DecentEthRouter.sol 38 require(gasCurrencyIsEth, "Gas currency is not ETH"); 43 require( 51 require(weth.balanceOf(address(this)) > amount, "not enough reserves"); 62 require(balance >= amount, "not enough balance");
File: src/bridge_adapters/StargateBridgeAdapter.sol 157 require(
File: src/swappers/UniSwapper.sol 96 require(uniswap_router != address(0), "router not set");
https://github.com/code-423n4/2024-01-decent/blob/main/src/swappers/UniSwapper.sol#L137
File: src/UTB.sol 75 require(msg.value >= swapParams.amountIn, "not enough native");
https://github.com/code-423n4/2024-01-decent/blob/main/src/UTB.sol#L76
File: src/UTBFeeCollector.sol 29 require(signature.length == 65, "Invalid signature length"); 54 require(recovered == signer, "Wrong signature");
https://github.com/code-423n4/2024-01-decent/blob/main/src/UTBFeeCollector.sol#L10
Use ASSEMBLY for small keccak256 hashes, in order to save gas.If the arguments to the encode call can fit into the scratch space (two words or fewer),then it's more efficient to use assembly to generate the hash (80 gas)
keccak256(abi.encodePacked(x, y)) -> assembly {mstore(0x00, a); mstore(0x20, b); let hash := keccak256(0x00, 0x40); }
File: src/UTBFeeCollector.sol 49 bytes32 constructedHash = keccak256( 50 abi.encodePacked(BANNER, keccak256(packedInfo))
https://github.com/code-423n4/2024-01-decent/blob/main/src/UTBFeeCollector.sol#L49
#0 - raymondfam
2024-01-26T16:51:33Z
All findings are generically known to complement the bot report.
#1 - c4-pre-sort
2024-01-26T16:51:38Z
raymondfam marked the issue as sufficient quality report
#2 - alex-ppg
2024-02-04T17:56:08Z
These findings were penalized:
storage
#3 - c4-judge
2024-02-04T17:56:14Z
alex-ppg marked the issue as grade-b
🌟 Selected for report: fouzantanveer
Also found by: 0xAadi, 0xSmartContract, 0xepley, SAQ, SBSecurity, albahaca, catellatech, clara, foxb868, hunter_w3b, ihtishamsudo, yongskiws
23.4096 USDC - $23.41
Decent allows users to perform transactions across multiple blockchain networks (like Ethereum, Polygon, Optimism etc.) in a single click, even if they hold funds/tokens on a different chain.
UTB and Decent Bridge.
UTB handles routing the cross-chain transaction and passes it to the appropriate bridge. It supports functions like swapAndExecute (for on-chain swaps) and bridgeAndExecute (for cross-chain transactions).
Decent Bridge is built on LayerZero's OFT standard and acts as the actual bridge, facilitating the transfer of tokens/funds from one chain to another.
Swappers: These are modules that perform same-chain transactions. They must implement the ISwapper interface.
Bridge Adapters: These are modules that facilitate cross-chain transactions. They must implement the IBridgeAdapter interface. Examples include DecentBridgeAdapter and StargateBridgeAdapter.
Decent simplifies the process of making transactions across various blockchains, allowing users to use different tokens and providing flexibility in payment methods. The platform is built on specific libraries and standards to ensure compatibility and extendability.
The key contracts of Decent protocol for this Audit are:
As the audit of these contracts, I checked for potential security vulnerabilities, such as reentrancy, access control issues, and logic flaws. Additionally, thoroughly test the functions and roles defined in these contracts to make sure they behave as expected.
UTB.sol: The UTB
contract facilitates one-click cross-chain transactions by coordinating token swaps, fee collection, and interaction with multiple blockchain networks through swappers and bridge adapters in the Decent protocol.
UTBExecutor.sol: This contract extending the Owned contract, enables the execution of payment transactions involving native or ERC20 tokens, handling transfers
, approvals
, and refunds
, with flexibility for additional gas or native fees in the Decent protocol.
UTBFeeCollector.sol: UTBFeeCollector contract inheriting from UTBOwned, facilitates the collection
and verification
of fees, supporting both native
and ERC20
tokens, with a specified signer for fee structure verification in the Decent protocol.
BaseAdapter.sol: The BaseAdapter
contract establishes a foundation for bridge
adapters in the Decent protocol, providing a mechanism to set the bridge executor
and a modifier to restrict certain functions to be callable only by the designated bridge executor.
DecentBridgeAdapter.sol: Serves as a bridge adapter in the Decent protocol, facilitating token transfers between different blockchain networks using the Decent protocol, with customizable fee estimation and bridging
functionality.
StargateBridgeAdapter.sol:This contract is a bridge adapter for the Stargate protocol, enabling cross-chain token transfers with customizable
fee estimation and integration
with the Stargate network.
UniSwapper.sol: The UniSwapper
contract, inheriting from UTBOwned and implementing the ISwapper interface, serves as a decentralized exchange adapter for UniSwap, enabling token swaps with customizable parameters, fee estimation, and seamless integration into the Universal Token Bridge (UTB
) ecosystem.
DcntEth.sol: The contract is extending the OFTV2 token contract, represents a decentralized token on the Ethereum blockchain named "Decent Eth" (with symbol "DcntEth") that supports minting and burning functionalities, and is associated with a Decent Eth Router for controlled access to these operations.
DecentEthRouter.sol: The contract is an Ethereum router facilitating cross-chain transactions with DecentEth tokens, supporting the transfer and call functionalities, and managing reserves of WETH
for bridging operations between Ethereum and other chains.
DecentBridgeExecutor.sol: This contract is serving as an executor for cross-chain transactions, handling the execution of transactions with either WETH
or native Ether, based on the gas currency configuration, and facilitating calls to target contracts.
Here are the key roles
Owner Role:
onlyOwner
modifier.Router Role:
onlyRouter
modifier.LzApp Role:
onlyLzApp
modifier.User Roles:
userDepositing
and userIsWithdrawing
are used to manage user-specific actions.Privileged Roles
Some privileged roles exercise powers over the contracts:
modifier onlyExecutor() { require( msg.sender == address(bridgeExecutor), "Only bridge executor can call this" ); _; }
modifier onlyRouter() { require(msg.sender == router); _; }
modifier onlyLzApp() { require(address(dcntEth) == msg.sender, "DecentEthRouter: only lz App can call"); _; }
These roles help define the permissions and access levels within the protocol, ensuring that only authorized entities can perform certain operations. The roles contribute to the overall security and functionality of the decentralized application by controlling access to critical functions and resources.
Accordingly, I analyzed and audited the subject in the following steps;
Core Protocol Contract Overview:
I focused on thoroughly understanding the codebase and providing recommendations to improve its functionality. The main goal was to take a close look at the important contracts and how they work together in the Decent Protocol.
I start with the following contracts, which play crucial roles in the Decent protocol:
Main Contracts I Looked At
I start with the following contracts, which play crucial roles in the Decent protocol:
UTB.sol DecentBridgeAdapter.sol StargateBridgeAdapter.sol UniSwapper.sol DecentEthRouter.sol
I started my analysis by examining the UTB.sol
contract, as this contract contains the core logic for Decent, enabling one-click transactions across different blockchain networks through functions like performSwap, swapAndExecute, and bridgeAndExecute
Then, I turned my attention to the DecentBridgeAdapter.sol
contract a critical component of Decent, which extends the BaseAdapter and IBridgeAdapter interfaces, facilitating cross-chain transactions by interacting with the Decent Eth Router, estimating fees, and managing the bridging of funds and execution of transactions across different chains.
The StargateBridgeAdapter
contract is an integral part of Decent, extends the BaseAdapter, IBridgeAdapter, and IStargateReceiver interfaces, facilitating cross-chain transactions via the Stargate protocol, managing the bridging of funds and execution of transactions across different chains, estimating fees, and interacting with the Stargate Router through functions like bridge, getValue, and sgReceive.
The UniSwapper
contract, an essential component of the Decent protocol, extends the UTBOwned and ISwapper interfaces, providing functionality for swapping tokens using Uniswap's V3 Router, handling different swap scenarios, and interacting with the specified Uniswap router to facilitate token swaps with features like swap, swapNoPath, swapExactIn, and swapExactOut.
Documentation Review:
Then went to Review This Documentation for a more detailed and technical explanation of Decent protocol.
Compiling code and running provided tests:
To setup the repo, first run forge install + pnpm i
To run the tests, simply add the relevant files to your .env, referencing .env.example, then run forge test
Manuel Code Review In this phase, I initially conducted a line-by-line analysis, following that, I engaged in a comparison mode.
Line by Line Analysis: Pay close attention to the contract's intended functionality and compare it with its actual behavior on a line-by-line basis.
Comparison Mode: Compare the implementation of each function with established standards or existing implementations, focusing on the function names to identify any deviations.
Overall, I consider the quality of the Decent protocol codebase to be Good. The code appears to be mature and well-developed. We have noticed the implementation of various standards adhere to appropriately. Details are explained below:
Codebase Quality Categories | Comments |
---|---|
Code Maintainability and Reliability | The Decent Protocol contracts demonstrates good maintainability through modular structure, consistent naming. It also prioritizes reliability by handling errors, conducting security checks. However, for enhanced reliability, consider professional security audits and documentation improvements. Regular updates are crucial in the evolving space. |
Code Comments | During the audit of the Decent contracts codebase, I found that some areas lacked sufficient internal documentation to enable independent comprehension. While comments provided high-level context for certain constructs, lower-level logic flows and variables were not fully explained within the code itself. Following best practices like NatSpec could strengthen self-documentation. As an auditor without additional context, it was challenging to analyze code sections without external reference docs. To further understand implementation intent for those complex parts, referencing supplemental documentation was necessary. |
Documentation | The documentation of the Decent project is quite comprehensive and detailed, providing a solid overview of how Bridging is structured and how its various aspects function. However, we have noticed that there is room for additional details, such as diagrams, to gain a deeper understanding of how different contracts interact and the functions they implement. With considerable enthusiasm. We are confident that these diagrams will bring significant value to the protocol as they can be seamlessly integrated into the existing documentation, enriching it and providing a more comprehensive and detailed understanding for users, developers and auditors. |
Testing | The audit scope of the contracts to be reviewed is 75%, with the aim of reaching 100% to increase the safety. |
Code Structure and Formatting | The codebase contracts are well-structured and formatted. but some order of functions does not follow the Solidity Style Guide According to the Solidity Style Guide, functions should be grouped according to their visibility and ordered: constructor, receive, fallback, external, public, internal, private. Within a grouping, place the view and pure functions last. |
Error | Use custom errors, 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. |
The analysis provided highlights several significant systemic and centralization risks present in the Decent protocol. These risks encompass concentration risk in UTB
, UTBExecutor
risk and more, third-party dependency risk, and centralization risks arising.
Here's an analysis of potential systemic and centralization risks in the contracts:
UTB.sol:
performSwap
and callBridge
, are made without checking the return values. A failed external call might leave the contract in an inconsistent state. It's important to handle potential failures properly.UTBFeeCollector.sol
StargateBridgeAdapter.sol
value
variable in the callBridge
function involves multiplication and division operations with potentially large numbers. Ensure that these calculations are precise and do not result in unintended rounding errors.UniSwapper.sol
UTB.sol:
Single Owner Control: The contract is owned by a single address (Owned), creating centralization risks. The owner has significant control over critical functions such as setting the executor, wrapped token, fee collector, registering swappers, and bridge adapters.
Fee Collection Centralization: The fee collection mechanism (feeCollector.collectFees
) is central to the contract, relying on a single fee collector contract. If this fee collector becomes compromised or malfunctions, it could impact the entire fee collection process.
Bridge Adapter Registration: The registration of bridge adapters is centralized, allowing only the owner to add or modify bridge adapters. This creates a single point of control and potential risk if the owner becomes malicious or compromised.
UTBFeeCollector.sol
Single Owner Control: The contract is owned by a single address (UTBOwned
), creating centralization risks. The owner has significant control over critical functions, such as setting the signer and redeeming fees.
Signer Configuration: The signer address, crucial for fee verification, is set by the owner. If the owner is compromised or acts maliciously, they can set a malicious signer, compromising the entire fee collection process.
RedeemFees Function: The ability to redeem fees is centralized in the owner. This function allows the owner to withdraw collected fees, and its execution is solely dependent on the owner's decision.
DecentBridgeAdapter.sol
destinationBridgeAdapter
) is centralized and controlled by the owner. If the owner is compromised or acts maliciously, they could register a malicious destination bridge adapter.Properly managing these risks and implementing best practices in security and decentralization will contribute to the sustainability and long-term success of the Decent protocol.
Here are the key points I would include in a conclusion:
Overall the code quality and architecture of the Decent protocol contracts are good. The code is well structured, tested and follows solidity best practices.
Some opportunities for improvement include adding more internal documentation, error handling, formal verification, and upgrading to support future changes.
There are some Centralization risks arise from single owner control over critical settings in contracts like UTB, reliance on single fee collector/signer, and centralized bridge adapter registration.
Systemic risks like external call vulnerabilities and value calculation precision issues could impact the protocol.
Following best practices in security, decentralization and ongoing maintenance/audits will help ensure the long-term sustainability and success of the protocol.
It is also highly recommended that the team continues to invest in security measures such as mitigation reviews, audits, and bug bounty programs to maintain the security and reliability of the project.
15 hours
#0 - c4-pre-sort
2024-01-26T17:56:34Z
raymondfam marked the issue as sufficient quality report
#1 - c4-judge
2024-02-03T14:49:47Z
alex-ppg marked the issue as grade-b