Platform: Code4rena
Start Date: 21/06/2022
Pot Size: $55,000 USDC
Total HM: 29
Participants: 88
Period: 5 days
Judge: gzeon
Total Solo HM: 7
Id: 134
League: ETH
Rank: 44/88
Findings: 2
Award: $144.81
🌟 Selected for report: 0
🚀 Solo Findings: 0
🌟 Selected for report: defsec
Also found by: 0x1f8b, 0x29A, 0xDjango, 0xNazgul, 0xNineDec, 0xf15ers, 0xkowloon, 0xmint, Bnke0x0, BowTiedWardens, Chom, ElKu, Funen, GalloDaSballo, GimelSec, IllIllI, JC, Kenshin, Kulk0, Lambda, Limbooo, MadWookie, Metatron, Picodes, Soosh, StErMi, TomJ, WatchPug, Waze, Yiko, _Adam, ak1, asutorufos, aysha, bardamu, catchup, datapunk, delfin454000, dipp, fatherOfBlocks, grGred, hake, hansfriese, hyh, joestakey, kebabsec, kenzo, kirk-baird, oyc_109, pashov, poirots, rfa, robee, saian, sashik_eth, shenwilly, simon135, slywaters, z3s, zeesaw, zer0dot
65.8003 USDC - $65.80
#1 Modifier must be first Impact modifier must be first running first
Proof of Concept https://github.com/code-423n4/2022-06-illuminate/blob/92cbb0724e594ce025d6b6ed050d3548a38c264b/marketplace/MarketPlace.sol#L247
Tool Used Manual review
Recommendation Mitigation Steps move authorized modifier be first before other function.
#2 Missing define natspec param aave Impact lend function have natspec comment which is missing the avee function parameter. Issues with comments are low risk based on Code4rena risk categories.
Proof of Concept https://github.com/code-423n4/2022-06-illuminate/blob/912be2a90ded4a557f121fe565d12ec48d0c4684/lender/Lender.sol#L536
Tool Used Manual review
Recommended Mitigation Steps Add natspec comments include avee parameter in lend function.
#3 Must be immutable
Impact the swivelAddr, apwineAddr, and lender can't be initialized by constructor.
Proof of Concept https://github.com/code-423n4/2022-06-illuminate/blob/912be2a90ded4a557f121fe565d12ec48d0c4684/redeemer/Redeemer.sol#L33
Tool Used Manual review
Recommended Mitigation Steps the state must add immutable because in the constructor parameter mention swivelAddr, apwineAddr, and lender to initialize. so i suggest to add immutable on it.
#4 Grammar Impact decrase readibility
Proof of Concept https://github.com/code-423n4/2022-06-illuminate/blob/912be2a90ded4a557f121fe565d12ec48d0c4684/redeemer/Redeemer.sol#L299
Tool Used Manual Review
Recommendation Mitigation Steps change from
/// @param a address that msg.sender must be to be authorized
to
/// @param a address that msg.sender must be authorized
🌟 Selected for report: BowTiedWardens
Also found by: 0v3rf10w, 0x1f8b, 0x29A, 0xKitsune, 0xNazgul, 0xf15ers, 0xkatana, 0xkowloon, Bnke0x0, ElKu, Fitraldys, Funen, GalloDaSballo, IllIllI, JC, Kaiziron, Lambda, MadWookie, Noah3o6, Nyamcil, RoiEvenHaim, TomJ, Tomio, UnusualTurtle, Waze, _Adam, ajtra, asutorufos, bardamu, c3phas, catchup, datapunk, defsec, delfin454000, fatherOfBlocks, grGred, hake, hansfriese, hyh, ignacio, joestakey, kebabsec, ladboy233, oyc_109, pashov, poirots, rfa, robee, sach1r0, samruna, sashik_eth, simon135, slywaters, z3s, zer0dot
79.0114 USDC - $79.01
#1 Visibility
change visibility from public to private or internal can save gas.
#2 Use type(uint).max
Remove max and replace max with type(uint256).max
uint256 max = 2**256 - 1; // approve the underlying for max per given principal for (uint8 i; i < 9; ) { // get the principal token's address address token = IMarketPlace(marketPlace).markets(u, m, i); // check that the token is defined for this particular market if (token != address(0)) { // max approve the token Safe.approve(IERC20(token), r, max);
to
// approve the underlying for max per given principal for (uint8 i; i < 9; ) { // get the principal token's address address token = IMarketPlace(marketPlace).markets(u, m, i); // check that the token is defined for this particular market if (token != address(0)) { // max approve the token Safe.approve(IERC20(token), r, type(uint256).max);
#3 Pre incement
use pre increment e.g ++i because pre-increment more cheaper gas than post increment e.g i++. i suggest to use pre increment.
#4 Use Calldata instead memory
In the external functions where the function argument is read-only, the function() has an inputed parameter that using memory, if this function didnt change the parameter, its cheaper to use calldata then memory. so we suggest to change it.
#5 Looping (aritmathic)
default uint is 0 so remove unnecassary explicit can reduce gas. caching the array length can reduce gas it caused access to a local variable is more cheap than query storage / calldata / memory in solidity. pre increment e.g ++i more cheaper gas than post increment e.g i++. i suggest to use pre increment.
#6 use story instead memory
Use storage instead of memory to reduce the gas fee. I suggest change from memory to storage.
#7.Caching Safe.tansferFrom
cache the Safe.tansferFrom because use multiple times. cache it can reduce the gas fee.
#8 Caching IElementToken(principal)
cache the IElementToken(principal) because use multiple times. cache it can reduce the gas fee.
Like this e.g https://github.com/code-423n4/2022-06-illuminate/blob/912be2a90ded4a557f121fe565d12ec48d0c4684/lender/Lender.sol#L388
#9 Change constant to immutable
Change these expressions from constant to immutable and implement the calculation in the constructor If the variable was immutable instead: the calculation would only be done once at deploy time (in the constructor), and then the result would be saved and read directly at runtime rather than being recalculated.
#10 Caching ITempus(principal)
cache the ITempus(principal) because use multiple times. cache it can reduce the gas fee. Like this e.g https://github.com/code-423n4/2022-06-illuminate/blob/912be2a90ded4a557f121fe565d12ec48d0c4684/lender/Lender.sol#L388