Wrong `DOMAIN_TYPEHASH` definition

[hats-bug-reporter[bot]]

Github username: @Rotcivegaf Twitter username: rotcivegaf Submission hash (on-chain): 0x778dc107b91369f0e44e5fbd99b51edb2d9d32d27731be2f41e4e609d6f6dbd7 Severity: medium

Description: Lines:

• https://github.com/Satsyxbt/Fenix/blob/bfcee140a38b39f0eb34dea6921b3b7d5248df82/contracts/core/VotingEscrowUpgradeable.sol#L1116
• https://github.com/Satsyxbt/Fenix/blob/bfcee140a38b39f0eb34dea6921b3b7d5248df82/contracts/core/VotingEscrowUpgradeable.sol#L1327-L1329

Description:

In the build of the DOMAIN TYPEHASH the string version is forgotten, but the delegateBySig function, build the domainSeparator with the keccak256(bytes(version))

This broke the EIP 712, and the delegateBySig function

Attack Scenario:

Some contract or dapp/backend could building the DOMAIN_TYPEHASH with "right" struct(include the version) and try to use the delegateBySig function but this function will revert in the with the message "VotingEscrow::delegateBySig: invalid signature" because the expect DOMAIN_TYPEHASH in the VotingEscrow.sol contract was built with the "wrong" struct

Recommended Mitigation Steps:

Acording the EIP 712, in the Definition of domainSeparator:

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

Add string version, to the EIP712Domain string:

@@ -1113,7 +1113,7 @@ contract VotingEscrowUpgradeable is
     //////////////////////////////////////////////////////////////*/

     /// @notice The EIP-712 typehash for the contract's domain
-    bytes32 public constant DOMAIN_TYPEHASH = keccak256("EIP712Domain(string name,uint256 chainId,address verifyingContract)");
+    bytes32 public constant DOMAIN_TYPEHASH = keccak256("EIP712Domain(string name,string version,uint256 chainId,address verifyingContract)");

[BohdanHrytsak]

Thank you for the submission.

This issue is inherited from Thena and is independent of the changes we made, which makes it OOS.

(Possibly to be revised)

[rotcivegaf]

Will it be revised?

[BohdanHrytsak]

Since it is inherited from Thena/Chronos, and does not have critical consequences, but only to incorrectness in the construction of signatures, it remains outside the scope due to insufficient impact to be recognized in the scope