Platform: Code4rena
Start Date: 10/06/2021
Pot Size: $45,000 USDC
Total HM: 21
Participants: 12
Period: 7 days
Judge: LSDan
Total Solo HM: 13
Id: 13
League: ETH
Rank: 7/12
Findings: 3
Award: $1,428.56
🌟 Selected for report: 2
🚀 Solo Findings: 0
🌟 Selected for report: gpersoon
Also found by: maplesyrup, pauliax
maplesyrup
2 - Medium Risk
When running the analyzer code, the following functions were found in RCNftHubL2.sol to possibly lock funds due to it being a payable function with no withdraw function associated.
Contract locking ether found:
Contract RCNftHubL2
(contracts/nfthubs/RCNftHubL2.sol line(s)#15-239) has payable functions:
NativeMetaTransaction.executeMetaTransaction(address,bytes,bytes32,bytes32,uint8)
(contracts/lib/NativeMetaTransaction.sol line(s)#31-67)
According to Slither analyzer detector documentation (https://github.com/crytic/slither/wiki/Detector-Documentation#contracts-that-lock-ether)
Possible functions that receive funds with the payable attribute must have a withdraw function to secure that funds can be sent out from the function or remove payable attribute.
Although the function may not receive funds directly, there should be a withdraw function added to ensure that information needed from the function can be withdrawn safely or do not include payable attribute.
Console Output (Slither log):
INFO:Detectors: Contract locking ether found: Contract RCNftHubL2 (contracts/nfthubs/RCNftHubL2.sol#15-239) has payable functions: - NativeMetaTransaction.executeMetaTransaction(address,bytes,bytes32,bytes32,uint8) (contracts/lib/NativeMetaTransaction.sol#31-67) But does not have a function to withdraw the ether Reference: https://github.com/crytic/slither/wiki/Detector-Documentation#contracts-that-lock-ether
Solidity Compiler 0.8.4 Hardhat v2.3.3 Slither v0.8.0
Compiled, Tested, Deployed contracts on a local hardhat network.
Ran Slither-analyzer for further detecting and testing.
(Worked best under python venv)
#0 - Splidge
2021-06-15T14:50:22Z
Duplicate of #43
#1 - dmvt
2021-07-11T10:57:54Z
duplicate of #31
🌟 Selected for report: maplesyrup
Also found by: heiho1
585.971 USDC - $585.97
maplesyrup
2 - Medium Risk - Possible loss or lock of funds found in a function in the contract
When running the analyzer code, the following functions were found in RCOrderbook.sol to possibly lock funds due to it being a payable function with no withdraw function associated.
Contract locking ether found:
// contracts/RCOrderbook.sol // line(s) 15-876
Contract RCOrderbook
has payable functions:
// contracts/lib/NativeMetaTransaction.sol // line(s) 31-67
NativeMetaTransaction.executeMetaTransaction(address,bytes,bytes32,bytes32,uint8)
According to Slither analyzer detector documentation (https://github.com/crytic/slither/wiki/Detector-Documentation#contracts-that-lock-ether)
Possible functions that receive funds with the payable attribute must have a withdraw function to secure that funds can be sent out from the function or remove payable attribute.
Although the function may not receive funds directly, there should be a withdraw function added to ensure that information needed from the function can be withdrawn safely or do not include payable attribute.
Console Output (Slither log):
INFO:Detectors: Contract locking ether found: Contract RCOrderbook (contracts/RCOrderbook.sol#15-876) has payable functions: - NativeMetaTransaction.executeMetaTransaction(address,bytes,bytes32,bytes32,uint8) (contracts/lib/NativeMetaTransaction.sol#31-67) But does not have a function to withdraw the ether Reference: https://github.com/crytic/slither/wiki/Detector-Documentation#contracts-that-lock-ether
Solidity Compiler 0.8.4 Hardhat v2.3.3 Slither v0.8.0
Compiled, Tested, Deployed contracts on a local hardhat network.
Ran Slither-analyzer for further detecting and testing.
(Worked best under python venv)
#0 - Splidge
2021-06-15T09:14:14Z
I initially confirmed this because we aren't using the native currency on Matic/Polygon. However I think this should be disputed mainly because this function is used to call other functions which might be payable, although I admit currently we don't have payable functions, we might add them in the future. This library is used across all our contracts, had we put a payable function in the Treasury for instance, would this be considered a flaw to have this same library imported into the Orderbook?
#1 - Splidge
2021-06-15T14:51:35Z
Note that the duplicate issue #51 was submitted by the same user.
#2 - dmvt
2021-07-10T15:07:25Z
Agree with the sponsor's explanation, but the issue exists regardless. Adding a way to retrieve locked funds would mitigate the issue.
🌟 Selected for report: JMukesh
Also found by: 0xRajeev, a_delamo, cmichel, maplesyrup
maplesyrup
1 - Low Risk
According to the Slither-analyzer documentation (https://github.com/crytic/slither/wiki/Detector-Documentation#missing-zero-address-validation), the following functions need a way to verify that the address in variable is not 0x0 or is the correct address that ownership/funds need to be sent to. This makes sure that funds/ownership are not lost accidentally.
RCOrderbook.constructor(address,address)._factoryAddress
(contracts/RCOrderbook.sol line(s)#106)
lacks a zero-check on:
factoryAddress = _factoryAddress (contracts/RCOrderbook.sol line(s)#107)
Console output (Slither log):
INFO:Detectors: RCMarket.initialize(uint256,uint32[],uint256,uint256,address,address,address[],address,string)._artistAddress (contracts/RCMarket.sol#196) lacks a zero-check on : - artistAddress = _artistAddress (contracts/RCMarket.sol#229) RCMarket.initialize(uint256,uint32[],uint256,uint256,address,address,address[],address,string)._marketCreatorAddress (contracts/RCMarket.sol#199) lacks a zero-check on : - marketCreatorAddress = _marketCreatorAddress (contracts/RCMarket.sol#230) RCMarket.initialize(uint256,uint32[],uint256,uint256,address,address,address[],address,string)._affiliateAddress (contracts/RCMarket.sol#197) lacks a zero-check on : - affiliateAddress = _affiliateAddress (contracts/RCMarket.sol#231) RCOrderbook.constructor(address,address)._factoryAddress (contracts/RCOrderbook.sol#106) lacks a zero-check on : - factoryAddress = _factoryAddress (contracts/RCOrderbook.sol#107) RCOrderbook.constructor(address,address)._treasuryAddress (contracts/RCOrderbook.sol#106) lacks a zero-check on : - treasuryAddress = _treasuryAddress (contracts/RCOrderbook.sol#108) BridgeMockup.requireToPassMessage(address,bytes,uint256)._RCProxyAddress (contracts/mockups/BridgeMockup.sol#13) lacks a zero-check on : - (_success) = _RCProxyAddress.call{value: (0)}(_data) (contracts/mockups/BridgeMockup.sol#18) BridgeMockup.setProxyL1Address(address)._newAddress (contracts/mockups/BridgeMockup.sol#38) lacks a zero-check on : - oracleProxyMainnetAddress = _newAddress (contracts/mockups/BridgeMockup.sol#39) BridgeMockup.setProxyL2Address(address)._newAddress (contracts/mockups/BridgeMockup.sol#42) lacks a zero-check on : - oracleProxyXdaiAddress = _newAddress (contracts/mockups/BridgeMockup.sol#43) Reference: https://github.com/crytic/slither/wiki/Detector-Documentation#missing-zero-address-validation
Solidity Compiler 0.8.4 Hardhat v2.3.3 Slither v0.8.0
Compiled, Tested, Deployed contracts on a local hardhat network.
Ran Slither-analyzer for further detecting and testing.
(Worked best under python venv)
#0 - Splidge
2021-06-17T14:57:31Z
Duplicate of #56
#1 - dmvt
2021-07-11T10:36:21Z
duplicate of #56
🌟 Selected for report: maplesyrup
434.0526 USDC - $434.05
maplesyrup
1 - Low Risk
According to the Slither-analyzer documentation (https://github.com/crytic/slither/wiki/Detector-Documentation#local-variable-shadowing), shadowing local variables is naming conventions found in two or more variables that are similar. Although they do not pose any immediate risk to the contract, incorrect usage of the variables is possible and can cause serious issues if the developer does not pay close attention.
It is recommended that the naming of the following variables should be changed slightly to avoid any confusion:
RCOrderbook._updateBidInOrderbook(address,address,uint256,uint256,uint256,RCOrderbook.Bid)._owner
(contracts/RCOrderbook.sol line(s)#358) shadows:
Ownable._owner <------(state variable)
(node_modules/@openzeppelin/contracts/access/Ownable.sol line(s)#19)
RCOrderbook.closeMarket()._owner
(contracts/RCOrderbook.sol line(s)#639) shadows:
Ownable._owner <------(state variable)
(node_modules/@openzeppelin/contracts/access/Ownable.sol line(s)#19)
Solidity Compiler 0.8.4 Hardhat v2.3.3 Slither v0.8.0
Compiled, Tested, Deployed contracts on a local hardhat network.
Ran Slither-analyzer for further detecting and testing.
(Worked best under python venv)
#0 - Splidge
2021-06-21T14:41:32Z
fixed here