SNARKeling Treasure Hunt
First Flight #59
Beginner FriendlyGameFiFoundry
100 EXP
View results
Previous Next
Submission Details
Severity: low
Valid

Missing zero-address validation on UpdateVerifier method

sammy0x100

Missing zero address check on updateVerifier method leads to unpredictable behaviour

Description

  • The updateVerifier method allows th owner to update the verifier address incase of an issue

  • Since the zero address check is missing, the owner could accidentlly set the zero address as the new verifier which could lead to lock up of funds permanently and break all claims

/// @notice In case of a bug, allow the owner to update the verifier address.
function updateVerifier(IVerifier newVerifier) external {
//@audit missing zero address check for newVerifier
@>//no zero address check
require(paused, "THE_CONTRACT_MUST_BE_PAUSED");
require(msg.sender == owner, "ONLY_OWNER_CAN_UPDATE_VERIFIER");
verifier = newVerifier;
​
emit VerifierUpdated(address(newVerifier));
}

Risk

Likelihood:

  • As the comment says, when the owner wants to update the verifier, they will call this function to do so.

Impact:

  • Since there is no check whether the new verifier is the zero address, the owner can potentially set the zero address to be the new verifier and this could lead to permanent lock up of funds

Proof of Concept

The floowing PoC tests show that the issue will cause the contract to be permanently locked

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.27;
​
import {Test} from "forge-std/Test.sol";
import {TreasureHunt} from "../contracts/src/TreasureHunt.sol";
import {IVerifier} from "../contracts/src/Verifier.sol";
​
// Mock Verifier for testing
contract MockVerifier is IVerifier {
function verify(bytes calldata, bytes32[] calldata) external pure returns (bool) {
return true;
}
}
​
contract TreasureHuntUpdateVerifierTest is Test {
TreasureHunt treasureHunt;
MockVerifier mockVerifier;
address owner;
​
function setUp() public {
owner = address(this);
mockVerifier = new MockVerifier();
treasureHunt = new TreasureHunt(address(mockVerifier));
}
​
/// @notice PoC: updateVerifier() accepts address(0), breaking all future claims
function testUpdateVerifierWithZeroAddressBreaksContract() public {
// Step 1: Verify initial state - verifier is set correctly
assert(treasureHunt.getVerifier() != address(0), "Initial verifier should not be zero");
​
// Step 2: Pause the contract (required for updateVerifier)
treasureHunt.pause();
assertTrue(treasureHunt.isPaused(), "Contract should be paused");
​
// Step 3: Owner accidentally (or maliciously) updates verifier to address(0)
// This should have been rejected but is NOT checked
treasureHunt.updateVerifier(IVerifier(address(0)));
​
// Step 4: Verify the vulnerability - verifier is now address(0)
assertEq(treasureHunt.getVerifier(), address(0), "Verifier should now be address(0)");
​
// Step 5: Unpause the contract to resume operations
treasureHunt.unpause();
assertFalse(treasureHunt.isPaused(), "Contract should be unpaused");
​
// Step 6: Try to claim with a valid proof
bytes32 treasureHash = keccak256("treasure1");
address recipient = address(0xAAAA);
​
// This will fail because calling .verify() on address(0) will revert
// The contract becomes permanently stuck and unusable
vm.prank(address(0xBBBB));
vm.expectRevert();
treasureHunt.claim(
abi.encode(uint256(1)), // dummy proof
treasureHash,
payable(recipient)
);
}
​
/// @notice PoC: Demonstrates the impact - contract becomes unrecoverable
function testContractBecomesPermanentlyBroken() public {
// Fund the contract
treasureHunt.fund{value: 100 ether}();
​
// Pause and update to zero address
treasureHunt.pause();
treasureHunt.updateVerifier(IVerifier(address(0)));
​
// Unpause
treasureHunt.unpause();
​
// Now the contract is broken - no one can claim anymore
// The only way to fix would be to redeploy
​
// Attempting any claim will fail
bytes32 treasureHash = keccak256("treasure1");
address participant = address(0xCCCC);
​
vm.prank(participant);
vm.expectRevert();
treasureHunt.claim(
abi.encode(uint256(1)),
treasureHash,
payable(participant)
);
}
​
/// @notice PoC: Shows that constructor properly validates, but updateVerifier doesn't
function testConstructorValidatesZeroAddressButUpdateDoesNot() public {
// Constructor rejects address(0)
vm.expectRevert(TreasureHunt.InvalidVerifier.selector);
new TreasureHunt(address(0));
​
// But updateVerifier does NOT reject it - INCONSISTENT VALIDATION
treasureHunt.pause();
treasureHunt.updateVerifier(IVerifier(address(0))); // ✅ This should fail but doesn't
​
assertEq(treasureHunt.getVerifier(), address(0), "Inconsistent validation!");
}
​
/// @notice PoC: Shows how this could happen accidentally
function testAccidentalZeroAddressUpdate() public {
address newVerifier = address(0); // Typo or mistake
​
treasureHunt.pause();
​
// No validation - owner can't catch the mistake
treasureHunt.updateVerifier(IVerifier(newVerifier));
​
// Contract is now broken forever
treasureHunt.unpause();
​
// Withdraw is also stuck because contract is paused during update
// AND all future claims are impossible
assertTrue(treasureHunt.isPaused() == false);
​
bytes memory dummyProof = abi.encode(uint256(1));
bytes32 treasureHash = keccak256("treasure1");
​
vm.prank(address(0xDDDD));
vm.expectRevert();
treasureHunt.claim(dummyProof, treasureHash, payable(address(0xDDDD)));
}
}

Recommended Mitigation

Simply check if the newVerifer is the zero address and revert if it is not.

// @notice In case of a bug, allow the owner to update the verifier address.
function updateVerifier(IVerifier newVerifier) external {
if (address(newVerifier) == address(0)) revert InvalidVerifier;// add this
require(paused, "THE_CONTRACT_MUST_BE_PAUSED");
require(msg.sender == owner, "ONLY_OWNER_CAN_UPDATE_VERIFIER");
verifier = newVerifier;
​
emit VerifierUpdated(address(newVerifier));
}
​
Updates

Lead Judging Commences

s3mvl4d Lead Judge 18 days ago
Submission Judgement Published
Validated
Assigned finding tags:

no zero-address check in updateVerifier()

The issue is that `updateVerifier()` allows the owner to replace the verifier with an arbitrary address, including `address(0)`, even though the constructor explicitly treats a zero verifier as invalid and reverts with `InvalidVerifier()` during initial deployment. In other words, the contract establishes at deployment time that a null verifier address is not an acceptable configuration, but then fails to preserve that same invariant when the verifier is later updated through the admin recovery path.

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.

Give us feedback!