Semaphore: Missing Smart Contract Range Check

From WEB3 Vulnerapedia
Jump to navigation Jump to search

Semaphore: Missing Smart Contract Range Check

Identified By: PSE Security Team

The Semaphore smart contracts performed range checks in some places but not others. The range checks were to ensure that all public inputs were less than the snark scalar field order. However, these checks weren’t enforced in all the necessary places. This could cause new Semaphore group owners to unknowingly create a group that will always fail verification.

Background

Semaphore is a DApp built on Ethereum that allows users to prove their membership of a group and send signals such as votes or endorsements without revealing their original identity. Essentially, trusted coordinators create a group (with a group Id) and add members via the smart contracts. Members can then submit zero knowledge proofs to the coordinator that prove their membership of the group and optionally contain a signal with it.

The Vulnerability

Since the Solidity uint256 type can hold numbers larger than the snark scalar field order, it is important to be weary of overflows. In order to prevent unwanted overflows, the Semaphore verifier smart contract automatically fails if a public input is greater than the snark scalar field order:

 // From Semaphore/contracts/base/Verifier.sol (outdated)
 require(input[i] < snark_scalar_field, "verifier-gte-snark-scalar-field");

When a coordinator creates a new group, they can input any valid uint256 value as the id. This is a problem since the id is a public input for the zk proof. If the id is greater than the snark scalar field order, the verifier will always revert and the group isn’t operable.

The Fix

To prevent an inoperable group from being created, a check on the group Id is needed. This check needs to ensure that the new group id will be less than the snark scalar field order:

 // From Semaphore/contracts/base/SemaphoreGroups.sol
 function _createGroup(
     uint256 groupId,
     uint8 depth,
     uint256 zeroValue
   ) internal virtual {
 		// The Fix is the following require statement:
     require(groupId < SNARK_SCALAR_FIELD, "SemaphoreGroups: group id must be < SNARK_SCALAR_FIELD");
     require(getDepth(groupId) == 0, "SemaphoreGroups: group already exists");
 
     groups[groupId].init(depth, zeroValue);
     emit GroupCreated(groupId, depth, zeroValue);
   }

Related Vulnerabilities

Arithmetic Over/Under Flow vulnerability

Sources

Reported Github Issue

Commit of the Fix

https://github.com/0xPARC/zk-bug-tracker#semaphore-1