Platform: Code4rena
Start Date: 22/09/2023
Pot Size: $100,000 USDC
Total HM: 15
Participants: 175
Period: 14 days
Judge: alcueca
Total Solo HM: 4
Id: 287
League: ETH
Rank: 114/175
Findings: 1
Award: $25.68
🌟 Selected for report: 0
🚀 Solo Findings: 0
🌟 Selected for report: MrPotatoMagic
Also found by: 0xAadi, 0xDING99YA, 0xDemon, 0xRstStn, 0xSmartContract, 0xStriker, 0xWaitress, 0xbrett8571, 0xfuje, 0xsagetony, 0xsurena, 33BYTEZZZ, 3docSec, 7ashraf, ABA, ABAIKUNANBAEV, Aamir, Audinarey, Bauchibred, Black_Box_DD, Daniel526, DanielArmstrong, DanielTan_MetaTrust, Dinesh11G, Eurovickk, Franklin, Inspecktor, John, Jorgect, Joshuajee, K42, Kek, Koolex, LokiThe5th, MIQUINHO, Myd, NoTechBG, QiuhaoLi, SanketKogekar, Sathish9098, Sentry, Soul22, SovaSlava, Stormreckson, Tendency, Topmark, Udsen, V1235816, Viktor_Cortess, Viraz, Yanchuan, ZdravkoHr, Zims, albahaca, albertwh1te, alexweb3, alexxander, ast3ros, audityourcontracts, bareli, bin2chen, bronze_pickaxe, c0pp3rscr3w3r, cartlex_, castle_chain, chaduke, debo, ether_sky, gumgumzum, imare, its_basu, jaraxxus, jasonxiale, josephdara, kodyvim, ladboy233, lanrebayode77, lsaudit, mert_eren, minhtrng, n1punp, nadin, niroh, nmirchev8, orion, peakbolt, perseverancesuccess, pfapostol, ptsanev, rvierdiiev, saneryee, shaflow2, te_aut, terrancrypt, twcctop, unsafesol, ustas, versiyonbir, windhustler, yongskiws, zhaojie, ziyou-
25.6785 USDC - $25.68
Issue | Instances | |
---|---|---|
[L‑01] | Missing Event for critical functions or parameters change | 4 |
[L‑02] | Missing access control in RootBridgeAgentFactory.sol#createBridgeAgent | 1 |
Issue | Instances | |
---|---|---|
[N‑01] | _rootBridgeAgentFactoryAddress parameter not used in ArbitrumBranchBridgeAgentFactory.sol#createBridgeAgent function | 2 |
[N‑02] | Use Address cannot be the zero address instead of Address cannot be 0 in require statements error messages | 23 |
[N‑03] | Typos | 1 |
[N‑04] | Unused error definition | 4 |
[N‑05] | Follow Solidity standard naming conventions (internal function style rule) for functions | 2 |
Events help non-contract tools to track changes, and events prevent users from being surprised by changes.
There is 5 instance of this issue:
File: src/factories/ERC20hTokenBranchFactory.sol 96: function createToken(string memory _name, string memory _symbol, uint8 _decimals, bool _addPrefix)
GitHub: 96
File: src/factories/ERC20hTokenRootFactory.sol 76: function createToken(string memory _name, string memory _symbol, uint8 _decimals)
GitHub: 76
File: src/VirtualAccount.sol 51: function withdrawNative(uint256 _amount) external override requiresApprovedCaller { 52: msg.sender.safeTransferETH(_amount); 53: } 56: function withdrawERC20(address _token, uint256 _amount) external override requiresApprovedCaller { 57: _token.safeTransfer(msg.sender, _amount); 58: } 61: function withdrawERC721(address _token, uint256 _tokenId) external override requiresApprovedCaller { 62: ERC721(_token).transferFrom(address(this), msg.sender, _tokenId); 63 }
GitHub: 51, 56, 61
RootBridgeAgentFactory.sol#createBridgeAgent
Due to the lack of access control mechanism in RootBridgeAgentFactory.sol#createBridgeAgent
Anybody can create bridge agents, and this cause the bridgeAgents
to grow endlessly.
There is 1 instance of this issue:
File: src/factories/RootBridgeAgentFactory.sol 48: function createBridgeAgent(address _newRootRouterAddress) external returns (address newBridgeAgent) {
GitHub: 48
_rootBridgeAgentFactoryAddress
parameter not used in ArbitrumBranchBridgeAgentFactory.sol#createBridgeAgent
functionThe _rootBridgeAgentFactoryAddress
parameter is not used in the function logic and its not makes any changes in the logic. Its only used for a checking the state variable rootBridgeAgentFactoryAddress
. So this can be removed, its not make any effect on the business logic.
There are 2 instances of this issue:
File: src/factories/ArbitrumBranchBridgeAgentFactory.sol 79: function createBridgeAgent( 80: address _newBranchRouterAddress, 81: address _rootBridgeAgentAddress, 82: @> address _rootBridgeAgentFactoryAddress 83: ) external virtual override returns (address newBridgeAgent) { 84: require( 85: msg.sender == localCoreBranchRouterAddress, "Only the Core Branch Router can create a new Bridge Agent." 86: ); 87: require( 88: @> _rootBridgeAgentFactoryAddress == rootBridgeAgentFactoryAddress, 89: "Root Bridge Agent Factory Address does not match." 90: );
GitHub: 79
File: src/factories/BranchBridgeAgentFactory.sol 116: function createBridgeAgent( 117: address _newBranchRouterAddress, 118: address _rootBridgeAgentAddress, 119: @> address _rootBridgeAgentFactoryAddress 120: ) external virtual returns (address newBridgeAgent) { 121: require( 122: msg.sender == localCoreBranchRouterAddress, "Only the Core Branch Router can create a new Bridge Agent." 123: ); 124: require( 125: @> _rootBridgeAgentFactoryAddress == rootBridgeAgentFactoryAddress, 126: "Root Bridge Agent Factory Address does not match." 127: );
GitHub: 116
Address cannot be the zero address
instead of Address cannot be 0
in require statements error messagesContract sometimes uses Address cannot be the zero address
and sometimes Address cannot be 0
. To achieve a standard, its better to use Address cannot be the zero address
instead of Address cannot be 0
because its more meaningful and readable
There are 23 instances of this issue:
<details> <summary>see instances</summary>File: src/ArbitrumBranchPort.sol 39: require(_rootPortAddress != address(0), "Root Port Address cannot be 0");
GitHub: 39
File: src/BaseBranchRouter.sol 61: require(_localBridgeAgentAddress != address(0), "Bridge Agent address cannot be 0");
GitHub: 61
File: src/MulticallRootRouter.sol 93: require(_localPortAddress != address(0), "Local Port Address cannot be 0"); 94: require(_multicallAddress != address(0), "Multicall Address cannot be 0"); 110: require(_bridgeAgentAddress != address(0), "Bridge Agent Address cannot be 0");
File: src/RootPort.sol 130: require(_bridgeAgentFactory != address(0), "Bridge Agent Factory cannot be 0 address."); 131: require(_coreRootRouter != address(0), "Core Root Router cannot be 0 address."); 152: require(_coreRootBridgeAgent != address(0), "Core Root Bridge Agent cannot be 0 address."); 153: require(_coreLocalBranchBridgeAgent != address(0), "Core Local Branch Bridge Agent cannot be 0 address."); 154: require(_localBranchPortAddress != address(0), "Local Branch Port Address cannot be 0 address.");
GitHub: 130, 131, 152, 153, 154
File: src/factories/ArbitrumBranchBridgeAgentFactory.sol 57: require(_coreRootBridgeAgent != address(0), "Core Root Bridge Agent Address cannot be 0");
GitHub: 57
File: src/factories/BranchBridgeAgentFactory.sol 61: require(_rootBridgeAgentFactoryAddress != address(0), "Root Bridge Agent Factory Address cannot be 0"); 66: require(_localCoreBranchRouterAddress != address(0), "Core Branch Router Address cannot be 0"); 67: require(_localPortAddress != address(0), "Port Address cannot be 0"); 68: require(_owner != address(0), "Owner cannot be 0"); 88: require(_coreRootBridgeAgent != address(0), "Core Root Bridge Agent cannot be 0");
File: src/factories/ERC20hTokenBranchFactory.sol 43: require(_localPortAddress != address(0), "Port address cannot be 0"); 61: require(_coreRouter != address(0), "CoreRouter address cannot be 0");
File: src/factories/ERC20hTokenRootFactory.sol 35: require(_rootPortAddress != address(0), "Root Port Address cannot be 0"); 50: require(_coreRouter != address(0), "CoreRouter address cannot be 0");
File: src/factories/RootBridgeAgentFactory.sol 32: require(_rootPortAddress != address(0), "Root Port Address cannot be 0");
GitHub: 32
</details>File: src/token/ERC20hTokenRoot.sol 39: require(_rootPortAddress != address(0), "Root Port Address cannot be 0"); 40: require(_factoryAddress != address(0), "Factory Address cannot be 0");
There are 1 instances of this issue:
File: src/interfaces/IBranchBridgeAgent.sol // @audit `callOutSystem()` is an external function not internal 130: * @notice Internal function performs call to Layerzero Enpoint Contract for cross-chain messaging.
GitHub: 130
Note that there may be cases where an error superficially appears to be used, but this is only because there are multiple definitions of the error in different files. In such cases, the error definition should be moved into a separate file. The instances below are the unused definitions.
There are 4 instances of this issue:
File: src/interfaces/IBranchPort.sol 255: error NotEnoughDebtToRepay();
GitHub: 255
File: src/interfaces/IERC20hTokenBranchFactory.sol 34: error UnrecognizedPort();
GitHub: 34
File: src/interfaces/IERC20hTokenRoot.sol 57: error UnrecognizedPort();
GitHub: 57
File: src/interfaces/IPortStrategy.sol 29: error UnrecognizedPort();
GitHub: 29
The below codes don’t follow Solidity’s standard naming convention,
internal functions : the mixedCase format starting with an underscore (_mixedCase starting with an underscore)
There are 2 instances of this issue:
File: src/VirtualAccount.sol 147: function isContract(address addr) internal view returns (bool) {
GitHub: 147
File: src/RootPort.sol 359: function addVirtualAccount(address _user) internal returns (VirtualAccount newAccount) {
GitHub: 359
#0 - c4-pre-sort
2023-10-15T08:49:07Z
0xA5DF marked the issue as sufficient quality report
#1 - c4-judge
2023-10-20T13:21:11Z
alcueca marked the issue as grade-a