QA Report #27
Labels
bug
Something isn't working
QA (Quality Assurance)
Assets are not at risk. State handling, function incorrect as to spec, issues with clarity, syntax
missing checks for zero address
description
Checking addresses against zero-address during initialization or during setting is a security best-practice. However, such checks are missing in address variable initializations/changes in many places.
Impact: Allowing zero-addresses will lead to contract reverts and force redeployments if there are no setters for such address variables.
findings
https://github.com/code-423n4/2022-06-illuminate/blob/92cbb0724e594ce025d6b6ed050d3548a38c264b/marketplace/MarketPlace.sol#L55
https://github.com/code-423n4/2022-06-illuminate/blob/92cbb0724e594ce025d6b6ed050d3548a38c264b/marketplace/MarketPlace.sol#L102
https://github.com/code-423n4/2022-06-illuminate/blob/92cbb0724e594ce025d6b6ed050d3548a38c264b/marketplace/MarketPlace.sol#L127
https://github.com/code-423n4/2022-06-illuminate/blob/92cbb0724e594ce025d6b6ed050d3548a38c264b/redeemer/Redeemer.sol#L53-L56
https://github.com/code-423n4/2022-06-illuminate/blob/92cbb0724e594ce025d6b6ed050d3548a38c264b/redeemer/Redeemer.sol#L74
https://github.com/code-423n4/2022-06-illuminate/blob/92cbb0724e594ce025d6b6ed050d3548a38c264b/redeemer/Redeemer.sol#L85
https://github.com/code-423n4/2022-06-illuminate/blob/92cbb0724e594ce025d6b6ed050d3548a38c264b/redeemer/Redeemer.sol#L93
https://github.com/code-423n4/2022-06-illuminate/blob/92cbb0724e594ce025d6b6ed050d3548a38c264b/lender/Lender.sol#L67-L69
No Transfer Ownership Pattern
description
Recommend considering implementing a two step process where the admin nominates an account and the nominated account needs to call an acceptAdmin() function for the transfer of admin to fully succeed. This ensures the nominated EOA account is a valid and active account.
findings
https://github.com/code-423n4/2022-06-illuminate/blob/92cbb0724e594ce025d6b6ed050d3548a38c264b/marketplace/MarketPlace.sol#L109-L112
https://github.com/code-423n4/2022-06-illuminate/blob/92cbb0724e594ce025d6b6ed050d3548a38c264b/redeemer/Redeemer.sol#L62-L65
https://github.com/code-423n4/2022-06-illuminate/blob/92cbb0724e594ce025d6b6ed050d3548a38c264b/lender/Lender.sol#L129-L132
https://github.com/code-423n4/2022-06-illuminate/blob/92cbb0724e594ce025d6b6ed050d3548a38c264b/lender/Lender.sol#L157
use of magic numbers
description
constants should be declared rather than use magic numbers
findings
the parameter i is uint8, it uses the magic number 9 instead of defining a constant
https://github.com/code-423n4/2022-06-illuminate/blob/92cbb0724e594ce025d6b6ed050d3548a38c264b/lender/Lender.sol#L87
admin can set fee to any amount
description
there are no sanity checks to the fee that admin can set
https://github.com/code-423n4/2022-06-illuminate/blob/92cbb0724e594ce025d6b6ed050d3548a38c264b/lender/Lender.sol#L137-L140
admin can set critical parameters without timelock
description
the admin can set an arbitary address for the
marketPlace
andswivelAddr
this should be set in the constructor and not change to give users more trust
https://github.com/code-423n4/2022-06-illuminate/blob/92cbb0724e594ce025d6b6ed050d3548a38c264b/lender/Lender.sol#L145-L151
https://github.com/code-423n4/2022-06-illuminate/blob/92cbb0724e594ce025d6b6ed050d3548a38c264b/lender/Lender.sol#L157
use more descriptive variable names
the code base uses variable names such as p, u, m, a, y
which makes the code harder to understand when reviewing the code, recommend using more descriptive names to avoid unnecessary confusion
The text was updated successfully, but these errors were encountered: