Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

QA Report #262

Open
code423n4 opened this issue Jun 26, 2022 · 0 comments
Open

QA Report #262

code423n4 opened this issue Jun 26, 2022 · 0 comments
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

Comments

@code423n4
Copy link
Contributor

#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

https://github.com/code-423n4/2022-06-illuminate/blob/912be2a90ded4a557f121fe565d12ec48d0c4684/redeemer/Redeemer.sol#L300

https://github.com/code-423n4/2022-06-illuminate/blob/912be2a90ded4a557f121fe565d12ec48d0c4684/lender/Lender.sol#L741-L754

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

https://github.com/code-423n4/2022-06-illuminate/blob/912be2a90ded4a557f121fe565d12ec48d0c4684/redeemer/Redeemer.sol#L27

https://github.com/code-423n4/2022-06-illuminate/blob/912be2a90ded4a557f121fe565d12ec48d0c4684/redeemer/Redeemer.sol#L23

https://github.com/code-423n4/2022-06-illuminate/blob/912be2a90ded4a557f121fe565d12ec48d0c4684/lender/Lender.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

https://github.com/code-423n4/2022-06-illuminate/blob/912be2a90ded4a557f121fe565d12ec48d0c4684/lender/Lender.sol#L740

https://github.com/code-423n4/2022-06-illuminate/blob/912be2a90ded4a557f121fe565d12ec48d0c4684/marketplace/MarketPlace.sol#L246

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
@code423n4 code423n4 added bug Something isn't working QA (Quality Assurance) Assets are not at risk. State handling, function incorrect as to spec, issues with clarity, syntax labels Jun 26, 2022
code423n4 added a commit that referenced this issue Jun 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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
Projects
None yet
Development

No branches or pull requests

1 participant