Submission Details
Impact:
Likelihood:
Missing zero-address validation on UpdateVerifier method
Author Revealed upon completion
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
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));
}
Rewards breakdown
nSLOC
220This contest has a flat reward structure with the following payouts:
100 EXP
20 EXP
2 EXP
Live
The contest is live. Earn rewards by submitting a finding.
Appeals
This is your time to appeal against judgements on your submissions.
Appeals Review
Appeals are being carefully reviewed by our judges.
Rewards Distribution
The contest is complete and the rewards are being distributed.