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

Validations #84

Open
code423n4 opened this issue Oct 10, 2021 · 1 comment
Open

Validations #84

code423n4 opened this issue Oct 10, 2021 · 1 comment
Labels
2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Warden finding disagree with severity Sponsor confirms validity, but disagrees with warden’s risk assessment (sponsor explain in comments) sponsor acknowledged Technically the issue is correct, but we're not going to resolve it for XYZ reasons

Comments

@code423n4
Copy link
Contributor

Handle

pauliax

Vulnerability details

Impact

function setBondPercentDiv should validate that newBondPercentDiv is not 0, or bondForRebalance will experience division by zero error otherwise. If you want to allow 0 values, then bondForRebalance should accommodate for such a possibility.

function addBounty should check that amount > 0 to prevent empty bounties.

function setMinLicenseFee should validate that it is not over 100%: newMinLicenseFee <= BASE.

function mintTo should validate that 'to' is not an empty address (0x0) to prevent accidental loss of tokens.

function validateWeights should validate that token is not this basket erc20: require(_tokens[i] != address(this));

function proposeBasketLicense could validate that 'tokenName' and 'tokenSymbol' are not empty.

function function setBondPercentDiv should validate that newBondPercentDiv > 1, otherwise it may become impossible to bondBurn because then bondAmount = totalSupply and calculation of newIbRatio will produce division by zero runtime error. Of course, this value is very unlikely but still would be nice to enforce this algorithmically.

Recommended Mitigation Steps

Consider applying suggested validations to make the protocol more robust.

@code423n4 code423n4 added 2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Warden finding labels Oct 10, 2021
code423n4 added a commit that referenced this issue Oct 10, 2021
@frank-beard frank-beard added disagree with severity Sponsor confirms validity, but disagrees with warden’s risk assessment (sponsor explain in comments) sponsor acknowledged Technically the issue is correct, but we're not going to resolve it for XYZ reasons labels Oct 19, 2021
@GalloDaSballo
Copy link
Collaborator

I agree with the warden, adding these checks will provide additional safety guarantees to protocol users (by limiting owner privileges)

Additionally, some of these setters can be used to DOS the protocol, as such this is a valid medium severity finding

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Warden finding disagree with severity Sponsor confirms validity, but disagrees with warden’s risk assessment (sponsor explain in comments) sponsor acknowledged Technically the issue is correct, but we're not going to resolve it for XYZ reasons
Projects
None yet
Development

No branches or pull requests

3 participants