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

Open
code423n4 opened this issue Sep 1, 2022 · 0 comments
Open

QA Report #199

code423n4 opened this issue Sep 1, 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

code423n4 commented Sep 1, 2022

Overview

Risk Rating Number of issues
Low Risk 3
Non-Critical 1

Findings

[L-01] Make revokePolicyApprovals stricter

TreasuryCustodian.sol#L54

if (Policy(policy_).isActive()) revert PolicyStillActive();

As revokePolicyApprovals can be called by anyone, it is possible to revoke a non-policy contract that was given a treasury approval, if the contract has a public isActive function that returns false value.

Consider making the address validation stricter by also checking whether the address has a public kernel function which returns the same address as TreasuryCustodian's kernel.

[L-02] Missing zero address check in KernelAdapter constructor

Kernel.sol#L65-L67

constructor(Kernel kernel_) {
    kernel = kernel_;
}

A faulty deployment script might deploy a module/policy with zero address which would render the contract useless, incurring a gas cost for contract the re-deployment.

Consider adding a zero address check for kernel.

[L-03] Missing validation for cushionFactor

Operator.sol#L134
In Operator.constructor(), there is no check to make sure that cushionFactor/configParams[0] is within acceptable range (100 to 10000). A faulty deployment script might set a wrong value that could cause irregular behaviour during bond market creations.

Consider adding a check in constructor() to make sure the value is within acceptable range:

if (configParams[0] > 10000 || configParams[0] < 100) revert Operator_InvalidParams();

[N-01] Comment Typo

PRICE.sol#L126

// Cache numbe of observations to save gas.

should be:

// Cache number of observations to save gas.
@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 Sep 1, 2022
code423n4 added a commit that referenced this issue Sep 1, 2022
code423n4 added a commit that referenced this issue Sep 1, 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