Popcorn contest - Ada's results

Lines of code

https://github.com/code-423n4/2023-01-popcorn/blob/d95fc31449c260901811196d617366d6352258cd/src/vault/VaultController.sol#L666-L670

Vulnerability details

Impact

The function _verifyCreatorOrOwner(address vault) internal returns (VaultMetadata memory metadata) {} is used to verify that the caller is the creator of the vault or owner of VaultController (admin rights). However, the if statement logic mistakenly used || instead of using &&, which leads to the verification always reverts.

These functions listed below were affected.

  function addStakingRewardsTokens(address[] memory vaults, bytes[] memory rewardTokenData) public {}
  function pauseAdapters(address[] calldata vaults) external {}
  function pauseVaults(address[] calldata vaults) external {}
  function unpauseAdapters(address[] calldata vaults) external {}
  function unpauseVaults(address[] calldata vaults) external {}

Proof of Concept

Set the msg.sender to creator, the if statement is true (0 || 1 = 1) the verification reverts. Set the msg.sender to owner, the if statement is true (1 || 0 = 1) the verification reverts. Set the msg.sender to a random address, the if statement is true (1 || 1 = 1) the verification reverts.

Tools Used

Manual review.

Recommended Mitigation Steps

Change the if statement to the following code.

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