Centrifuge - nobody2018's results

Lines of code

centrifuge/blob/main/src/gateway/routers/axelar/Router.sol#L44

Vulnerability details

Impact

AxelarRouter.execute has the onlyCentrifugeChainOrigin modifier, which assumes msg.sender is axelarGateway. This is wrong because axelarGateway never does this. msg.sender is the relayer from Axelar. Therefore, this function will revert due to the check in onlyCentrifugeChainOrigin, which has two impacts:

1. The gas paid for all messages will be drained. This will lead to funds loss from protocol.
2. All messages cannot be processed, and the core functions of the protocol are significantly broken. Assets deposited by users can never be withdrawn.

Proof of Concept

According to axelar's docs, we can understand that cross-chain messages need to go through 3 phrases (6 steps). Let's see how messages are processed:

At the source chain

1. User (dApp) calls a callContract (or callContractWithToken) function on the Axelar Gateway contract to initiate a call. Once the call is initiated, the user can see its status at [https://axelarscan.io/gmp/txHash] or programmatically track it via the AxelarJS SDK.
2. User prepays the relayer gas fee on the source chain to Axelar’s Gas Services contract.
3. The call enters the Axelar Gateway from the source chain.

At the Axelar network

4. Axelar network confirms the call and converts the paid gas from the source chain’s native token into the destination chain’s native token.

At the destination chain

5. The call is approved and emerges from the Axelar Gateway on the destination chain.
6. The executor service relays and executes the approved call to the application’s Axelar Executable interface.

In Axelar Gateway, each message will calculated as a hash value via keccak256, which represents the stored slot. If the value of this slot is true, it means that the message can be executed. The 5th step is to set the slot corresponding to the message to true. Next, the executor service from Axelar (commonly known as relayer) calls the execute method of the target contract of the message. In this case, it's Router.execute.

File: src\gateway\routers\axelar\Router.sol
73:     function execute(
74:         bytes32 commandId,
75:         string calldata sourceChain,
76:         string calldata sourceAddress,
77:         bytes calldata payload
78:->   ) public onlyCentrifugeChainOrigin(sourceChain, sourceAddress) {
79:         bytes32 payloadHash = keccak256(payload);
80:         require(
81:             axelarGateway.validateContractCall(commandId, sourceChain, sourceAddress, payloadHash),
82:             "Router/not-approved-by-gateway"
83:         );
84: 
85:         gateway.handle(payload);
86:     }

43:     modifier onlyCentrifugeChainOrigin(string calldata sourceChain, string calldata sourceAddress) {
44:->       require(msg.sender == address(axelarGateway), "AxelarRouter/invalid-origin");
......
54:     }

So, at L44, tx will revert since msg.sender is not axelarGateway.

L80-83, This implementation method has guaranteed the validity of the message. axelarGateway.validateContractCall(commandId, sourceChain, sourceAddress, payloadHash) will internally check whether the slot corresponding to the message is true. If it is true, set it to false and return the original bool.

Tools Used

Manual Review

Recommended Mitigation Steps

Remove the check at L44.

Assessed type

DoS

Lines of code

https://github.com/code-423n4/2023-09-centrifuge/blob/main/src/InvestmentManager.sol#L291

Vulnerability details

Impact

Users can request decreasing the outstanding deposit orders via decreaseDepositRequest which will send a cross-chain message to the Centrifuge chain. After the message is processed, Centrifuge's bot will decrease the outstanding deposit orders and send another cross-chain message back to the source chain to notify that the request has been processed. This will eventually call handleExecutedDecreaseInvestOrder, which internally transfers the asset from escrow to user via asset.transferFrom. Assets can be tokens with blacklist mechanisms such as USDC and USDT. If the user is blacklisted, it will cause USDC.transferFrom revert. Therefore, the message cannot be successfully processed, which means that the user cannot obtain the decreased asset, resulting in a loss of funds.

Proof of Concept

When a message of type Call.ExecutedDecreaseInvestOrder is executed by relayer, the whole flow is as follows:

Router.execute	//src\gateway\routers\axelar\Router.sol
  gateway.handle
    investmentManager.handleExecutedDecreaseInvestOrder
      ......some checks skipped
      SafeTransferLib.safeTransferFrom(_currency, address(escrow), user, currencyPayout)

Let's take a look at the code of handleExecutedDecreaseInvestOrder:

File: src\InvestmentManager.sol
277:     function handleExecutedDecreaseInvestOrder(
278:         uint64 poolId,
279:         bytes16 trancheId,
280:         address user,
281:         uint128 currency,
282:         uint128 currencyPayout
283:     ) public onlyGateway {
284:         require(currencyPayout != 0, "InvestmentManager/zero-payout");
285: 
286:         address _currency = poolManager.currencyIdToAddress(currency);
287:         address liquidityPool = poolManager.getLiquidityPool(poolId, trancheId, _currency);
288:         require(liquidityPool != address(0), "InvestmentManager/tranche-does-not-exist");
289:         require(_currency == LiquidityPoolLike(liquidityPool).asset(), "InvestmentManager/not-tranche-currency");
290: 
291:->       SafeTransferLib.safeTransferFrom(_currency, address(escrow), user, currencyPayout);
292:     }

L291, if _currency is USDC or USDT and user is blacklisted, tx will revert.

Tools Used

Manual Review

Recommended Mitigation Steps

It is recommended not to transfer the asset directly to the user, but to record the amount of the asset and provide the claim function to allow the user to specify the receiver.

Assessed type

DoS