Cached DOMAIN_SEPARATOR is incorrect for tranche tokens potentially breaking permit integrations

[c4-submissions]

Lines of code

https://github.com/code-423n4/2023-09-centrifuge/blob/512e7a71ebd9ae76384f837204216f26380c9f91/src/util/Factory.sol#L81-L109
https://github.com/code-423n4/2023-09-centrifuge/blob/512e7a71ebd9ae76384f837204216f26380c9f91/src/token/ERC20.sol#L42-L49
https://github.com/code-423n4/2023-09-centrifuge/blob/512e7a71ebd9ae76384f837204216f26380c9f91/src/token/ERC20.sol#L225-L231

Vulnerability details

Impact

Attempts to interact with tranche tokens via permit may always revert.

Proof of Concept

When new tranche tokens are deployed, the initial DOMAIN_SEPARATOR is calculated and cached in the constructor.
https://github.com/code-423n4/2023-09-centrifuge/blob/512e7a71ebd9ae76384f837204216f26380c9f91/src/token/ERC20.sol#L42-L49

    constructor(uint8 decimals_) {
        ...
        deploymentChainId = block.chainid;
        _DOMAIN_SEPARATOR = _calculateDomainSeparator(block.chainid);
    }

This uses an empty string since name is only set after deployment.
https://github.com/code-423n4/2023-09-centrifuge/blob/512e7a71ebd9ae76384f837204216f26380c9f91/src/util/Factory.sol#L81-L109

    function newTrancheToken(
        uint64 poolId,
        bytes16 trancheId,
        string memory name,
        string memory symbol,
        uint8 decimals,
        address[] calldata trancheTokenWards,
        address[] calldata restrictionManagerWards
    ) public auth returns (address) {
        ...
        TrancheToken token = new TrancheToken{salt: salt}(decimals);

        token.file("name", name);
        ...
    }

Consequently, the domain separator is incorrect (when block.chainid == deploymentChainId where the domain separator is not recalculated) and will cause reverts when signatures for permit are attempted to be constructed using the tranche token's name (which will not be empty).

It should also be noted that the tranche token name could be changed by a call to updateTranchTokenMetadata which may also introduce complications with the domain separator.

Tools Used

Manual Review

Recommended Mitigation Steps

Consider setting the name in the constructor before the cached domain separator is calculated.

Assessed type

Other

[c4-pre-sort]

raymondfam marked the issue as primary issue

[c4-pre-sort]

raymondfam marked the issue as sufficient quality report

[c4-pre-sort]

raymondfam marked the issue as high quality report

[c4-sponsor]

hieronx (sponsor) confirmed

[c4-judge]

gzeon-c4 marked the issue as satisfactory

[c4-judge]

gzeon-c4 marked the issue as selected for report

[hieronx]

Mitigated in centrifuge/liquidity-pools#142