Centrifuge - Kow's results

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