Skip to content

Latest commit

 

History

History
63 lines (45 loc) · 2.57 KB

009.md

File metadata and controls

63 lines (45 loc) · 2.57 KB
Raw

Acidic Midnight Mustang

Medium

Protocol is not EIP712 compliant as it does not dynamically build domain separator.

Summary

In case of a hard fork, the block.chainId on which the contract operates can change. For this reason, the original OZ EIP712 contract has implemented their _domainSeparatorV4 function which checks if the chain id is still the same. In case it is not, a new domain separator is dynamically built.

    function _domainSeparatorV4() internal view returns (bytes32) {
        if (address(this) == _cachedThis && block.chainid == _cachedChainId) {
            return _cachedDomainSeparator;
        } else {
            return _buildDomainSeparator();
        }
    }

    function _buildDomainSeparator() private view returns (bytes32) {
        return keccak256(abi.encode(TYPE_HASH, _hashedName, _hashedVersion, block.chainid, address(this)));
    }

However, in VVVVCTokenDistributor and in VVVVCInvestmentLedger the domain separator is hardcoded and is always used

        // EIP-712 domain separator
        DOMAIN_SEPARATOR = keccak256(
            abi.encode(
                DOMAIN_TYPEHASH,
                keccak256(abi.encodePacked("VVV", _environmentTag)),
                block.chainid,
                address(this)
            )
        );

In case the chain the contract is deployed on makes a hard fork and the chain id changes, the contract will continue to use the old chain id in its domain separator, ultimately not complying to the EIP.

Marking the issue as Medium as the contracts are both expected to comply with EIP712 based on the readme

Q: Is the codebase expected to comply with any specific EIPs? Both VVVVCInvestmentLedger.sol and VVVVCTokenDistributor.sol define EIP-712 domain components and utilize EIP-712 structured data formats in validating signature used to validate calls to VVVVCInvestmentLedger:invest() and VVVVCTokenDistributor:claim().

Root Cause

Always using the same domain separator

Attack Path

  1. Protocol is deployed on chain X
  2. After some time, there is a hard fork of chain X. The chain Id is changed
  3. The contract now still expects signatures including the domain separator with the old chain id. The contract no longer complies with EIP712.

Affected Code

https://github.com/sherlock-audit/2024-11-vvv-exchange-update/blob/main/vvv-platform-smart-contracts/contracts/vc/VVVVCTokenDistributor.sol#L96

Impact

Not complying with an EIP when expected to.

Mitigation

Implement a similar function to OZ which dynamically builds the domain separator in case chain id changes.