Platform: Code4rena
Start Date: 18/10/2022
Pot Size: $75,000 USDC
Total HM: 27
Participants: 144
Period: 7 days
Judge: gzeon
Total Solo HM: 13
Id: 170
League: ETH
Rank: 57/144
Findings: 2
Award: $55.67
🌟 Selected for report: 0
🚀 Solo Findings: 0
🌟 Selected for report: Rolezn
Also found by: 0x1f8b, 0x52, 0x5rings, 0xNazgul, 0xSmartContract, 0xZaharina, 0xhunter, 0xzh, 8olidity, Amithuddar, Aymen0909, B2, Bnke0x0, Chom, Deivitto, Diana, Diraco, Dravee, Franfran, JC, Jeiwan, Josiah, JrNet, Jujic, KingNFT, KoKo, Lambda, Margaret, Migue, Ocean_Sky, PaludoX0, Picodes, Rahoz, RaoulSchaffranek, RaymondFam, RedOneN, ReyAdmirado, Shinchan, Tagir2003, Trust, Waze, Yiko, __141345__, a12jmx, adriro, ajtra, arcoun, aysha, ballx, bin2chen, bobirichman, brgltd, bulej93, catchup, catwhiskeys, caventa, cccz, cdahlheimer, ch0bu, chaduke, chrisdior4, cloudjunky, cryptostellar5, cryptphi, csanuragjain, cylzxje, d3e4, delfin454000, djxploit, durianSausage, erictee, fatherOfBlocks, francoHacker, gianganhnguyen, gogo, hansfriese, i_got_hacked, ignacio, imare, karanctf, kv, leosathya, louhk, lukris02, lyncurion, m_Rassska, malinariy, martin, mcwildy, mics, minhtrng, nicobevi, oyc_109, pashov, peanuts, pedr02b2, peiw, rbserver, ret2basic, rotcivegaf, rvierdiiev, ryshaw, sakman, sakshamguruji, saneryee, securerodd, seyni, sikorico, svskaushik, teawaterwire, tnevler, w0Lfrum
55.6726 USDC - $55.67
ecrecoverecrecover is vulnerable to signature malleability. So it is better to use ECDSA library to eliminate the risk of signature malleability.
https://github.com/code-423n4/2022-10-holograph/blob/main/contracts/HolographFactory.sol#L333-L334
Storage gap is necessary for upgradeable contracts, to prevent storage collision due to future developments. So according to openzeppelin it is recommended to add a storage gap of uint _gap[50].
Below upgradeable contracts are missing such storage gaps :
https://github.com/code-423n4/2022-10-holograph/blob/main/contracts/HolographOperator.sol#L125
https://github.com/code-423n4/2022-10-holograph/blob/main/contracts/HolographFactory.sol#L123
https://github.com/code-423n4/2022-10-holograph/blob/main/contracts/enforcer/Holographer.sol#L115
https://github.com/code-423n4/2022-10-holograph/blob/main/contracts/HolographBridge.sol#L122
Some critical functions which updates an important part of the protocol, should emit the updated record, to let the offchain users know about the change.
For example, in https://github.com/code-423n4/2022-10-holograph/blob/main/contracts/HolographBridge.sol#L502,
setOperator is a function to set the operator, which is a critical aspect of the protocol. Hence the function should emit to let the other users know about the change.
Also , timelocks are also needed , in case, where malicious operator is set, to give the users time , to act upon it.
Other such critical functions are : https://github.com/code-423n4/2022-10-holograph/blob/main/contracts/HolographOperator.sol#L278-L285 https://github.com/code-423n4/2022-10-holograph/blob/main/contracts/HolographBridge.sol#L472 https://github.com/code-423n4/2022-10-holograph/blob/main/contracts/HolographBridge.sol#L522
block.timestampblock.timestamp are riskier to use, as they can be manipulated by the miners. So any logic that depends on it, is vulnerable to manipulation.
Similarly other block attributes like block.number are also susceptible to miner manipulation. Hence we should avoid them.
https://github.com/code-423n4/2022-10-holograph/blob/main/contracts/HolographOperator.sol#L525 https://github.com/code-423n4/2022-10-holograph/blob/main/contracts/HolographOperator.sol#L1191 https://github.com/code-423n4/2022-10-holograph/blob/main/contracts/HolographOperator.sol#L345 https://github.com/code-423n4/2022-10-holograph/blob/main/contracts/enforcer/HolographERC20.sol#L469
require should have descriptive reasons on failure to enhance the readability of the source codehttps://github.com/code-423n4/2022-10-holograph/blob/main/contracts/enforcer/HolographERC20.sol#L606 https://github.com/code-423n4/2022-10-holograph/blob/main/contracts/enforcer/HolographERC20.sol#L502 https://github.com/code-423n4/2022-10-holograph/blob/main/contracts/enforcer/HolographERC20.sol#L484 https://github.com/code-423n4/2022-10-holograph/blob/main/contracts/enforcer/HolographERC721.sol#L391 https://github.com/code-423n4/2022-10-holograph/blob/main/contracts/enforcer/HolographERC721.sol#L373 https://github.com/code-423n4/2022-10-holograph/blob/main/contracts/enforcer/HolographERC721.sol#L460
indexed fieldsindexed field allows offchain monitoring tools to find the emitted event faster.
In https://github.com/code-423n4/2022-10-holograph/blob/main/contracts/enforcer/PA1D.sol#L153, all argument of SecondarySaleFees event are missing indexed field
TODO comments should be removed before going into production. So complete the TODO in https://github.com/code-423n4/2022-10-holograph/blob/main/contracts/HolographOperator.sol#L701, before deploying the project
#0 - alexanderattar
2022-11-09T21:19:56Z
Don't want to use __gap
🌟 Selected for report: oyc_109
Also found by: 0x040, 0x1f8b, 0x5rings, 0xNazgul, 0xSmartContract, 0xZaharina, 0xsam, 0xzh, 2997ms, Amithuddar, Aymen0909, B2, Bnke0x0, Deivitto, Diana, Dinesh11G, Franfran, JC, JrNet, Jujic, KingNFT, KoKo, Mathieu, Metatron, Mukund, Olivierdem, PaludoX0, Pheonix, Picodes, RaymondFam, RedOneN, ReyAdmirado, Rolezn, Saintcode_, Satyam_Sharma, Shinchan, Tagir2003, Tomio, Waze, Yiko, __141345__, adriro, ajtra, aysha, ballx, beardofginger, bobirichman, brgltd, bulej93, catchup, catwhiskeys, cdahlheimer, ch0bu, chaduke, chrisdior4, cryptostellar5, cylzxje, d3e4, delfin454000, dharma09, djxploit, durianSausage, emrekocak, erictee, exolorkistis, fatherOfBlocks, gianganhnguyen, gogo, halden, hxzy, i_got_hacked, iepathos, karanctf, leosathya, lucacez, lukris02, lyncurion, m_Rassska, martin, mcwildy, mics, nicobevi, peanuts, peiw, rbserver, ret2basic, rotcivegaf, ryshaw, sakman, sakshamguruji, saneryee, sikorico, skyle, svskaushik, tnevler, vv7, w0Lfrum, zishansami
0 USDC - $0.00
https://github.com/code-423n4/2022-10-holograph/blob/main/contracts/HolographOperator.sol#L218_L228 https://github.com/code-423n4/2022-10-holograph/blob/main/contracts/enforcer/HolographERC721.sol#L185-L196 https://github.com/code-423n4/2022-10-holograph/blob/main/contracts/enforcer/HolographERC20.sol#L156-L186
https://github.com/code-423n4/2022-10-holograph/blob/main/contracts/HolographOperator.sol#L804-L805 https://github.com/code-423n4/2022-10-holograph/blob/main/contracts/module/LayerZeroModule.sol#L256-L274 https://github.com/code-423n4/2022-10-holograph/blob/main/contracts/enforcer/HolographERC721.sol#L417-L426 https://github.com/code-423n4/2022-10-holograph/blob/main/contracts/module/LayerZeroModule.sol#L281-L293
require statement with multiple conditions should be splitted into multiple require statement to save gashttps://github.com/code-423n4/2022-10-holograph/blob/main/contracts/enforcer/HolographERC721.sol#L464-L469 https://github.com/code-423n4/2022-10-holograph/blob/main/contracts/HolographOperator.sol#L857
calldata instead of memory in read-only function arguments to save gashttps://github.com/code-423n4/2022-10-holograph/blob/main/contracts/HolographBridge.sol#L162 https://github.com/code-423n4/2022-10-holograph/blob/main/contracts/HolographOperator.sol#L240 https://github.com/code-423n4/2022-10-holograph/blob/main/contracts/enforcer/Holographer.sol#L147 https://github.com/code-423n4/2022-10-holograph/blob/main/contracts/enforcer/PA1D.sol#L173