Project
One World
NFTDeFi
15,000 USDC
View results
Previous Next
Submission Details
Severity: medium
Invalid

Domain separator hash is not EIP 712 compliant

wellbyt3

Summary

EIP712Base has two issues with the way the domain separator is generated which can result in issues with integrators:

  1. EIP712_DOMAIN_TYPEHASH is missing chainId, but contains salt. ChainId should also be uint256, not bytes.

  2. Incorrect ordering in _setDomainSeperator - chainId should come before verifyingContract

Vulnerability Details

According to EIP 712 (https://eips.ethereum.org/EIPS/eip-712#definition-of-domainseparator):

  • string name the user readable name of signing domain, i.e. the name of the DApp or the protocol.

  • string version the current major version of the signing domain. Signatures from different versions are not compatible.

  • uint256 chainId the EIP-155 chain id. The user-agent should refuse signing if it does not match the currently active chain.

  • address verifyingContract the address of the contract that will verify the signature. The user-agent may do contract specific phishing prevention.

  • bytes32 salt an disambiguating salt for the protocol. This can be used as a domain separator of last resort.

...

The EIP712Domain fields should be the order as above, skipping any absent fields.

Right now, chainId comes after verifyingContract, when it should come before.

function _setDomainSeperator(string memory name, string memory version) internal {
domainSeperator = keccak256(
abi.encode(
EIP712_DOMAIN_TYPEHASH,
keccak256(bytes(name)),
keccak256(bytes(version)),
address(this),
bytes32(getChainId())
)
);
}

https://github.com/Cyfrin/2024-11-one-world/blob/1e872c7ab393c380010a507398d4b4caca1ae32b/contracts/meta-transaction/EIP712Base.sol#L28-L38

Also, EIP712_DOMAIN_TYPEHASH is missing chainId and instead contains salt:

bytes32 internal constant EIP712_DOMAIN_TYPEHASH = keccak256(
bytes(
"EIP712Domain(string name,string version,address verifyingContract,bytes32 salt)"
)
);

https://github.com/Cyfrin/2024-11-one-world/blob/1e872c7ab393c380010a507398d4b4caca1ae32b/contracts/meta-transaction/EIP712Base.sol#L14-L18

Impact

Protocol is not compliant with EIP 712, which will result in issues with integrators.

Tools Used

Manual review

Recommendations

Update EIP712_DOMAIN_TYPEHASH to:

- "EIP712Domain(string name,string version,address verifyingContract,bytes32 salt)"
+ "EIP712Domain(string name,string version,uint256 chainId, address verifyingContract)"

Update _setDomainSeperator:

function _setDomainSeperator(string memory name, string memory version) internal {
domainSeperator = keccak256(
abi.encode(
EIP712_DOMAIN_TYPEHASH,
keccak256(bytes(name)),
keccak256(bytes(version)),
+ getChainId(),
address(this),
- bytes32(getChainId())
)
);
}
Updates

Lead Judging Commences

0xbrivan2 Lead Judge 10 months ago
Submission Judgement Published
Invalidated
Reason: Known issue

Support

FAQs

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