code-423n4 / 2022-10-juicebox-findings

2 stars 0 forks source link

No access control for function deployDeletateFor #219

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/jbx-protocol/juice-nft-rewards/blob/f9893b1497098241dd3a664956d8016ff0d0efd0/contracts/JBTiered721DelegateDeployer.sol#L69

Vulnerability details

Impact

Detailed description of the impact of this finding. There is no access control for the deployDelegateFor function, so anyone can call this function and initialize all the parameters for a project. Moreover, the caller can also transfer the owner to a possible malicious new owner in line 100.

Proof of Concept

Provide direct links to all referenced code in GitHub. Add screenshots, logs, or any other relevant proof that illustrates the concept. https://github.com/jbx-protocol/juice-nft-rewards/blob/f9893b1497098241dd3a664956d8016ff0d0efd0/contracts/JBTiered721DelegateDeployer.sol#L69

Tools Used

Manual

Recommended Mitigation Steps

Add a modifier onlyOwner() so that only the owner of the contract can all this function. Refactor the code for ownership transfer as a two-step process: 1) proposer a new pending owner; 2) the new pending owner accepts the role.

trust1995 commented 1 year ago

Invalid. Permissionless deployDelegateFor is the intended design.

drgorillamd commented 1 year ago

This is the business case, users should be allowed to call and deploy their own delegate (and pick an owner)

c4-judge commented 1 year ago

Picodes marked the issue as unsatisfactory: Invalid