Centrifuge - 0xfuje's results

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

Lines of code

https://github.com/code-423n4/2023-09-centrifuge/blob/main/src/gateway/Gateway.sol#L285-L366

Vulnerability details

Impact

Various impacts depending on the type of the transaction: e.g.: extra tokens can be gained, tranche can be updated to a deprecated price which can be exploited

Description

From Axelar Message Parsing Docs:

Occasionally, transactions can get “stuck” in the pipeline from a source to destination chain (e.g. due to one-off issues that arise with relayers that operate on top of the network).

Transactions have typically gotten “stuck” in the pipeline due to: (A) The transaction failing to relay from the source chain into the Axelar network for processing. (B) The transaction failing to get executed on the destination chain.

For example in the case of B, the transaction can be manually executed on the destination chain by anyone or anyone can increase the gas payment by sending it to the gas receiver of Axelar to replay these transactions.

The problem is while at the original sent time the cross-chain transaction was relevant, it's possible to do damage with it to the system at a later time. Two malicious examples are: updating tranche token price to an old value and transferring tokens that were already sent back to the user on source chain via an admin.

src/gateway/Gateway.sol - handle()

	function handle(bytes calldata message) external onlyIncomingRouter pauseable {
		...
		} else if (Messages.isUpdateTrancheTokenPrice(message)) {
            (uint64 poolId, bytes16 trancheId, uint128 currencyId, uint128 price) =
                Messages.parseUpdateTrancheTokenPrice(message);
            investmentManager.updateTrancheTokenPrice(poolId, trancheId, currencyId, price);
        } else if (Messages.isTransfer(message)) {
            (uint128 currency, address recipient, uint128 amount) = Messages.parseIncomingTransfer(message);
            poolManager.handleTransfer(currency, recipient, amount);
        }
        ...
	}

Deprecated price update

1. LiquidityPool.sol has a price of 100_000
2. In the past there were a failed transaction that were intended to change the price to 10_000
3. Malicious actor pays gas to Axelar relayer to replay the failed transaction
4. Price gets updated to 10_000
5. Malicious actor now can exploit the price in various ways (e.g. can deposit into LiquidityPool and get 10x more shares in the next epoch)

Cross-chain transfer

1. Malicious actor initiated a cross-chain transaction transfer() on PoolManager.sol
2. currency gets transferred to escrow on source chain
3. The cross-chain transaction fails, so he requests to get his tokens back from escrow from Centrifuge
4. Admin who has auth on Root rescues from escrow and sends the tokens back to him
5. Malicious actor replays the failed transaction by increasing the gas payment
6. Now he has double amount of tokens, half on source chain, half on destination chain

Tools Used

Manual review, Axelar Docs

Recommended Mitigation Steps

Consider monitoring the failed transactions, determining what is an appropriate time when a failed cross-chain transaction can still be replayed and not cause any harm. If they are in the appropriate timeframe: replay the transactions via the planned centrifuge relay gas bot. If they are out of time: call validateContractCall() on Axelar Gateway with the transaction's params to prevent the transaction from any possible replaying.

Assessed type

Other