Project

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

The protocol does not follow EIP-712 when setting domain Separator

Summary

The _setDomainSeperator method in EIP712Base.sol contract does not follow EIP-712 specification.

Vulnerability Details

The _setDomainSeperator() method is defined as :

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()) //@audit-issue the order does not follow EIP712.
)
);
}

as it can be seen address(this) comes before bytes32(getChainId()). However the expected order specified by EIP-712 should be as follow:

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.
Future extensions to this standard can add new fields with new user-agent behaviour constraints. User-agents are free to use the provided information to inform/warn users or refuse signing. Dapp implementers should not add private fields, new fields should be proposed through the EIP process.

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

Also getChainId() is typecasted to bytes32 and function name is spelled incorrectly . While these are not an issue, it’s worth noting that typecasting getChainId() to bytes32 is unnecessary. Since chainId is an atomic type, it can be directly encoded without any modifications.

Impact

Due to the use of incorrect order, the resultingdomainSeperator will not be the same as one calculated using the correct field order. This discrepancy would cause any signatures created using this domain separator to be incompatible with signatures expected by other EIP-712-compliant applications or contracts that follow the specified field order.

Tools Used

Recommendations

I suggest you to follow the expected implementation that specified by the EIP-712. Adjust the code as follow:

domainSeperator = keccak256(
abi.encode(
EIP712_DOMAIN_TYPEHASH,
keccak256(bytes(name)),
keccak256(bytes(version)),
getChainId(),
address(this)
)
);
Updates

Lead Judging Commences

0xbrivan2 Lead Judge about 1 year ago
Submission Judgement Published
Invalidated
Reason: Known issue

Appeal created

bhunter Submitter
about 1 year ago
0xbrivan2 Lead Judge
about 1 year ago
bhunter Submitter
about 1 year ago
0xbrivan2 Lead Judge
about 1 year ago
0xbrivan2 Lead Judge about 1 year ago
Submission Judgement Published
Invalidated
Reason: Known issue

Support

FAQs

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

Give us feedback!