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

Open
code423n4 opened this issue Jul 14, 2022 · 0 comments
Open

QA Report #640

code423n4 opened this issue Jul 14, 2022 · 0 comments
Labels
bug Warden finding old-submission-method QA (Quality Assurance) Assets are not at risk. State handling, function incorrect as to spec, issues with clarity, syntax sponsor acknowledged Technically the issue is correct, but we're not going to resolve it for XYZ reasons

Comments

@code423n4
Copy link
Contributor

few LOW / QA findings:

[QA-01] TransferOwnership has no zero-address check

Notably this also allows to create a contract with locked access:
deployFor
createFor

Results in: emit TransferOwnership(_oldOwner: VaultFactory: [0x037fc82298142374d974839236d2e2df6b5bdd8f], _newOwner: 0x0000000000000000000000000000000000000000)

[LOW-02] Vault.Sol: fallback allows to execute any installed method to be executable

Any methods that gets installed as a plugin can be executed without hash permission as the call directs to the internal function.
This may become dangerous if exposed method was meant to be callable by owner/permissioned module.

[QA-03] deploy/deployFor is callable directly

a direct call to VaultFactory does not make a register VaultRegistry

This may be intended to be deployable directly, but a new deploy will be occured by any msg.sendercaller in that case.

[LOW-04] createInCollection allows passing any token's address, which can be a fake FERC1155 or invalid

As the function expects token to pass a FERC1155, it allows setuping a vault with any token as registry.
An arbitrary token may allow to more attack control for that affected vault.

This may be intended, but it's preferable to not allow user to specify address of a token that is controllable by attacker and can be exploited in Buyout and Migration

[LOW-05] createCollectionFor can setup a token controller with zero address

a controller will not be able to fulfill onlyController checks in a token if controller address was set 0.

[QA-06] deployVault can be deployed with 0 fractionSupply minted

@code423n4 code423n4 added bug Warden finding old-submission-method QA (Quality Assurance) Assets are not at risk. State handling, function incorrect as to spec, issues with clarity, syntax labels Jul 14, 2022
code423n4 added a commit that referenced this issue Jul 14, 2022
@mehtaculous mehtaculous added the sponsor acknowledged Technically the issue is correct, but we're not going to resolve it for XYZ reasons label Jul 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Warden finding old-submission-method QA (Quality Assurance) Assets are not at risk. State handling, function incorrect as to spec, issues with clarity, syntax 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

2 participants