PoolManager assumes only one EVM address exists for all currencies

[c4-submissions]

Lines of code

https://github.com/code-423n4/2023-09-centrifuge/blob/512e7a71ebd9ae76384f837204216f26380c9f91/src/PoolManager.sol#L235-L255

Vulnerability details

Impact

All currencies are mapped to one EVM address, which should not be the case as some currencies have different addresses on different EVM chains

Proof of Concept

Take a look at the PoolManager.sol#L235-L255

    /// @notice A global chain agnostic currency index is maintained on Centrifuge. This function maps a currency from the Centrifuge index to its corresponding address on the evm chain.
    ///         The chain agnostic currency id has to be used to pass currency information to the Centrifuge.
    /// @dev    This function can only be executed by the gateway contract.
    function addCurrency(uint128 currency, address currencyAddress) public onlyGateway {
        //@audit-issue
        // Currency index on the Centrifuge side should start at
        require(currency != 0, "PoolManager/currency-id-has-to-be-greater-than-0");
        require(currencyIdToAddress[currency] == address(0), "PoolManager/currency-id-in-use");
        require(currencyAddressToId[currencyAddress] == 0, "PoolManager/currency-address-in-use");
        require(IERC20(currencyAddress).decimals() <= MAX_CURRENCY_DECIMALS, "PoolManager/too-many-currency-decimals");

        currencyIdToAddress[currency] = currencyAddress;
        currencyAddressToId[currencyAddress] = currency;

        // Enable taking the currency out of escrow in case of redemptions
        EscrowLike(escrow).approve(currencyAddress, investmentManager.userEscrow(), type(uint256).max);

        // Enable taking the currency out of escrow in case of decrease invest orders
        EscrowLike(escrow).approve(currencyAddress, address(investmentManager), type(uint256).max);

        emit CurrencyAdded(currency, currencyAddress);
    }

As seen from code implementation and the comments it's seen that currencies are only linked to one EVM address which is wrong, since some currencies currently have more than one EVM address, i.e on different addresses for the same currency on different chains with multiple popular stable coins being an example of this, which makes this an issue since interacting with the same currency on different EVM chains could lead to unexpected nuances.

Tool used

Manual Review

Recommended Mitigation Steps

Mapping of currencies to their EVM addresses needs to be refactored and take into account the fact that some currencies have more than one EVM address

Assessed type

Context

[c4-pre-sort]

raymondfam marked the issue as low quality report

[raymondfam]

Informational low, but will let the sponsor look into it.

[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 duplicate of #477

[c4-judge]

gzeon-c4 changed the severity to QA (Quality Assurance)