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

Open
code423n4 opened this issue Jun 3, 2022 · 1 comment
Open

QA Report #149

code423n4 opened this issue Jun 3, 2022 · 1 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 resolved Finding has been patched by sponsor (sponsor pls link to PR containing fix) sponsor confirmed Sponsor agrees this is a problem and intends to fix it (OK to use w/ "disagree with severity")

Comments

@code423n4
Copy link
Contributor

No check on address zero can cause logic errors and lost of funds
https://github.com/code-423n4/2022-05-backd/blob/8121e5244ca29f87b0763d05a69e7fc654d14f45/protocol/contracts/tokenomics/InflationManager.sol#L221
https://github.com/code-423n4/2022-05-backd/blob/8121e5244ca29f87b0763d05a69e7fc654d14f45/protocol/contracts/tokenomics/InflationManager.sol#L221
https://github.com/code-423n4/2022-05-backd/blob/8121e5244ca29f87b0763d05a69e7fc654d14f45/protocol/contracts/zaps/PoolMigrationZap.sol#L52

Add more comments on burnFees function and natspec comments
https://github.com/code-423n4/2022-05-backd/blob/8121e5244ca29f87b0763d05a69e7fc654d14f45/protocol/contracts/RewardHandler.sol#L35

Very hard to read code and its very packed together.its so compacted that my vscode visual extension cant read if its state or memory variable
https://github.com/code-423n4/2022-05-backd/blob/8121e5244ca29f87b0763d05a69e7fc654d14f45/protocol/contracts/RewardHandler.sol#L36-L48
—--------------------------------------
if (IERC20(token).allowance(address(this), spender) > 0) return;
IERC20(token).safeApprove(spender, type(uint256).max);
Just space it out to make it more readable
https://github.com/code-423n4/2022-05-backd/blob/8121e5244ca29f87b0763d05a69e7fc654d14f45/protocol/contracts/RewardHandler.sol#L63

https://github.com/code-423n4/2022-05-backd/blob/8121e5244ca29f87b0763d05a69e7fc654d14f45/protocol/contracts/zaps/PoolMigrationZap.sol#L52-L66
if allowance is more then zero then it will return nothing and first return something to help a function that calls this function
if this allowance is more than zero it will return nothing and it you cant approve anything its dead code

@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 3, 2022
code423n4 added a commit that referenced this issue Jun 3, 2022
@chase-manning chase-manning added sponsor confirmed Sponsor agrees this is a problem and intends to fix it (OK to use w/ "disagree with severity") resolved Finding has been patched by sponsor (sponsor pls link to PR containing fix) labels Jun 6, 2022
@GalloDaSballo
Copy link
Collaborator

Agree with observation about address 0 and adding comments.
Rest is opinion, and in lack of a suggested refactoring this is a low quality submission

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 resolved Finding has been patched by sponsor (sponsor pls link to PR containing fix) sponsor confirmed Sponsor agrees this is a problem and intends to fix it (OK to use w/ "disagree with severity")
Projects
None yet
Development

No branches or pull requests

3 participants