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

Missing chain ID in domain separator causes non-compliance with EIP-712 standard

whitehat777

Summary

The OneWorld Project aims to support the EIP-712 standard for typed data hashing and signing. However, the current implementation in the EIP712Base.sol contract does not fully comply with the standard, which could lead to unexpected issues.

Vulnerability Details

The vulnerability arises in the EIP712Base.sol contract, where the chainId is omitted from the domain separator hash. According to the EIP-712 standard, the domain separator should include the chainId to prevent certain security risks and ensure compatibility with other systems.

The relevant section in EIP712Base.sol is as follows:

EIP712Base.sol#L16

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

As we can see chainId is omitted in DOMAIN_TYPEHASH. As well as we can see that while setting the domain separator itself we again omit the chainId

EIP712Base.sol#L28-L38

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())
)
);
}

Thus the protocol does not follow the standard’s approach of directly including chainId in the domain separator, which can impact compatibility with other compliant protocols and tools.

Impact

Not following the standard can result in several issues:

  • Interoperability Issues: Tools and protocols that expect strict EIP-712 compliance might not work correctly with this contract.

  • Broken Integrations: Applications that use this contract may experience issues when interacting with wallets or other dApps designed around full EIP-712 compliance.

  • Without chainId in the domain separator, the contract could be vulnerable to replay attacks if deployed on multiple chains, even with salt derived from chainId. While this approach provides some protection, it is not the recommended method for preventing replay attacks, and attackers could exploit this vulnerability. Therefore, it is advised to follow the standard recommendation to include chainId in the domain separator to ensure robust protection against replay attacks.

Tools Used

Manual code review

Recommendations

Add chainId to the domain separator to fully comply with the standard. The updated code would look like this:

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 12 months ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

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