DOMAIN_SEPARATOR of ERC20 is incorrectly calculated and used

[c4-submissions]

Lines of code

https://github.com/code-423n4/2023-09-centrifuge/blob/main/src/token/ERC20.sol#L48
https://github.com/code-423n4/2023-09-centrifuge/blob/main/src/token/ERC20.sol#L71

Vulnerability details

DOMAIN_SEPARATOR of ERC20 is incorrectly calculated and used

Impact

permit() of ERC20 doesn't work as intented, implementation of EIP-1271 _isValidSignature() will use incorrect digest

Description

_DOMAIN_SEPARATOR of ERC20 is incorrectly calculated. _DOMAIN_SEPARATOR is used as DOMAIN_SEPARATOR in permit() and passed in the _isValidSignature() function.

Since block.chainId will always equal deploymentChainId as we see in the constructor: this means in permit() always the incorrectly calculated _DOMAIN_SEPARATOR will be used since the expression block.chainid == deploymentChainId ? _DOMAIN_SEPARATOR is always true.

src/token/ERC.20.sol - constructor

	constructor(uint8 decimals) {
		...
		deploymentChainId = block.chainId
		_DOMAIN_SERATOR = _calculateDomainSeparator(block.chainid)
	}

src/token/ERC20.sol - _calculateDomainSeparator()

    function _calculateDomainSeparator(uint256 chainId) private view returns (bytes32) {
        return keccak256(
            abi.encode(
                keccak256("EIP712Domain(string name,string version,uint256 chainId,address verifyingContract)"),
                keccak256(bytes(name)),
                keccak256(bytes(version)),
                chainId,
                address(this)
            )
        );
    }

The problem is that the _calculateDomainSeparator() will use the empty name when it is not yet set in the constructor, only later via file(). Even after the name was set, the incorrectly calculated _DOMAIN_SEPARATOR (set in constructor) remains to be used.

Tools Used

Manual review

Recommended Mitigation Steps

Set name in the constructor before calculating _DOMAIN_SEPARATOR via _calculateDomainSeparator(). Consider setting symbol in the constructor as well for convenience.

Assessed type

Timing

[c4-pre-sort]

raymondfam marked the issue as sufficient quality report

[c4-pre-sort]

raymondfam marked the issue as duplicate of #146

[c4-judge]

gzeon-c4 marked the issue as satisfactory