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 #242

Open
code423n4 opened this issue Aug 15, 2022 · 0 comments
Open

QA Report #242

code423n4 opened this issue Aug 15, 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 Manager and ve must be immutable

https://github.com/code-423n4/2022-08-fiatdao/blob/fece3bdb79ccacb501099c24b60312cd0b2e4bb2/contracts/features/Blocklist.sol#L11-L12

the state manager and ve can't be initialize by constructor. the constructor parameter mention state manager and ve to initialize. so i suggest to add immutable on manager and ve.

#2 Token, owner and penaltyRecipient must be immutable

https://github.com/code-423n4/2022-08-fiatdao/blob/fece3bdb79ccacb501099c24b60312cd0b2e4bb2/contracts/VotingEscrow.sol#L49-L50

https://github.com/code-423n4/2022-08-fiatdao/blob/fece3bdb79ccacb501099c24b60312cd0b2e4bb2/contracts/VotingEscrow.sol#L45

the state Token, owner and penaltyRecipient can't be initialize by constructor. the constructor parameter mention state Token, owner and penaltyRecipient to initialize. so i suggest to add immutable on Token, owner and penaltyRecipient.

#3 Missing natspec comment

https://github.com/code-423n4/2022-08-fiatdao/blob/fece3bdb79ccacb501099c24b60312cd0b2e4bb2/contracts/features/Blocklist.sol#L37

isContract() was missing natspec comment. add natspec comment to isContract() to give knowledge to the user about the function and params

#4 Missing indexed field

https://github.com/code-423n4/2022-08-fiatdao/blob/fece3bdb79ccacb501099c24b60312cd0b2e4bb2/contracts/VotingEscrow.sol#L39-L41

Each event should use indexed fields to reach clarity. add indexed in owner, blocklist, and recipient.

#5 Missing check for address

https://github.com/code-423n4/2022-08-fiatdao/blob/fece3bdb79ccacb501099c24b60312cd0b2e4bb2/contracts/VotingEscrow.sol#L100

constructor have three params address, so to avoid vulnerability we suggest to add simple check for the params
Add simple check e.g

               require( _owner != address(0), "address cant be 0");
               require( _blocklist != address(0), "address cant be 0");
               require( _recipient != address(0), "address cant be 0");
@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 Aug 15, 2022
code423n4 added a commit that referenced this issue Aug 15, 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