The domain separator is not calculated properly

[c4-submissions]

Lines of code

https://github.com/code-423n4/2023-09-centrifuge/blob/512e7a71ebd9ae76384f837204216f26380c9f91/src/token/ERC20.sol#L42-L49

Vulnerability details

Impact

The domain separator is not properly calculated in the constructor - as it does not take into calculation name - which is not yet initialized.

The similar issue had been evaluated as Medium during the previous Code4rena audits:

• Wrong DOMAIN_SEPARATOR 2022-05-rubicon-findings#38

Moreover, please notice, that this issue is not the same as my other report: #306
The current issue is about incorrectly setting the domain separator on contract deployment. The root cause of this issue lies inside constructor(), as domain separator is being initialized before name.
While the other issue (#306) is about not updating domain separator, when name is changed. The root cause of that issue lies inside file() - whenever we call to change the name, the domain separator should be updated too.
The remediation for this issue is to update constructor(), while remediation for issue #306 is to update code inside file().

Proof of Concept

File: src/token/ERC20.sol

    constructor(uint8 decimals_) {
        decimals = decimals_;
        wards[_msgSender()] = 1;
        emit Rely(_msgSender());

        deploymentChainId = block.chainid;
        _DOMAIN_SEPARATOR = _calculateDomainSeparator(block.chainid);
    }

The constructor() calculates the domain separator, before initializing name.

Tools Used

Manual code review

Recommended Mitigation Steps

Initialize name before calculating domain separator

Assessed type

Other

[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