Popcorn contest - georgits's results

Lines of code

https://github.com/code-423n4/2023-01-popcorn//blob/main/src/vault/VaultController.sol#L669

Vulnerability details

Impact

In the _verifyCreatorOrOwner description the caller has to be either creator of the vault OR owner of VaultController but the function reverts if the caller is not both creator of the vault AND owner of VaultController at the same time. All functions that call _verifyCreatorOrOwner will fail if msg.sender isn't creator of the vault AND owner of VaultController.

Proof of Concept

if (msg.sender != metadata.creator || msg.sender != owner) revert NotSubmitterNorOwner(msg.sender);

https://github.com/code-423n4/2023-01-popcorn//blob/main/src/vault/VaultController.sol#L669

Tools Used

Manual code review

Recommended Mitigation Steps

Change the if statement to

msg.sender != metadata.creator && msg.sender != owner

Use latest Solidity version with a stable pragma statement

All solidity files

Missing a check if the clone has already been registered

CloneRegistry.sol - 41

NatSpec must be updated

DeploymentController.sol - 60

Remove unused imports

YearnAdapter.sol - 6(IStrategy, IAdapter)

BeefyAdapter.sol - 6(IStrategy)

Use of ecrecover is susceptible to signature malleability, consider using OpenZeppelin's ECDSA library

MultiRewardStaking.sol - 459

Vault.sol - 678

AdapterBase.sol - 646