Cross-chain message authentication can be bypassed, allowing an attacker to disrupt the state of vaults
A malicious actor may send a cross-chain message to an `XProvider` contract and bypass the `onlySource` authentication check. As a result, they'll be able to call any function in the `XProvider` contract that has the `onlySource` modifier and disrupt the state of `XChainController` and all vaults.\\nThe protocol integrates with Connext to handle cross-chain interactions. `XProvider` is a contract that manages interactions between vaults deployed on all supported networks and `XChainController`. `XProvider` is deployed on each of the network where a vault is deployed and is used to send and receive cross-chain messages via Connext. `XProvider` is a core contract that handles vault rebalancing, transferring of allocations from Game to `XChainController` and to vaults, transferring of tokens deposited to vaults between vault on different networks. Thus, it's critical that the functions of this contract are only called by authorized actors.\\nTo ensure that cross-chain messages are sent from authorized actors, there's onlySource modifier that's applied to the xReceive function. The modifier checks that the sender of a message is trusted:\\n```\\nmodifier onlySource(address _originSender, uint32 _origin) {\\n  require(_originSender == trustedRemoteConnext[_origin] && msg.sender == connext, "Not trusted");\\n  _;\\n}\\n```\\n\\nHowever, it doesn't check that `trustedRemoteConnext[_origin]` is set (i.e. it's not the zero address), and `_originSender` can in fact be the zero address.\\nIn Connext, a message can be delivered via one of the two paths: the fast path or the slow path. The fast path is taken when, on the destination, message receiving is not authentication, i.e. when destination allows receiving of messages from all senders. The slow path is taken when message receiving on the destination is authenticated, i.e. destination allows any sender (it doesn't check a sender).\\nSince, `XProvider` always checks the sender of a message, only the slow path will be used by Connext to deliver messages to it. However, Connext always tries the slow path:\\nRouters observing the origin chain with funds on the destination chain will: Simulate the transaction (if this fails, the assumption is that this is a more "expressive" crosschain message that requires authentication and so must go through the AMB: the slow path).\\nI.e. it'll always send a message and see if it reverts on the destination or not: if it does, Connext will switch to the slow path.\\nWhen Connext executes a message on the destination chain in the fast path, it sets the sender address to the zero address:\\n```\\n(bool success, bytes memory returnData) = ExcessivelySafeCall.excessivelySafeCall(\\n  _params.to,\\n  gasleft() - Constants.EXECUTE_CALLDATA_RESERVE_GAS,\\n  0, // native asset value (always 0)\\n  Constants.DEFAULT_COPY_BYTES, // only copy 256 bytes back as calldata\\n  abi.encodeWithSelector(\\n    IXReceiver.xReceive.selector,\\n    _transferId,\\n    _amount,\\n    _asset,\\n    _reconciled ? _params.originSender : address(0), // use passed in value iff authenticated\\n    _params.originDomain,\\n    _params.callData\\n  )\\n);\\n```\\n\\nThus, Connext will try to call the `XProvider.xReceive` function with the `_originSender` argument set to the zero address. And there are situations when the `onlySource` modifier will pass such calls: when the origin network (as specified by the `_origin` argument) is not in the `trustedRemoteConnext` mapping.\\nAccording to the description of the project, it'll be deployed on the following networks:\\nMainnet, Arbitrum, Optimism, Polygon, Binance Smart Chain\\nAnd this is the list of networks supported by Connext:\\nEthereum Mainnet Polygon Optimism Arbitrum One Gnosis Chain BNB Chain\\nThus, a malicious actor can send a message from Gnosis Chain (it's not supported by Derby), and the `onlySource` modifier will pass the message. The same is true for any new network supported by Connext in the future and not supported by Derby.
In the `onlySource` modifier, consider checking that `trustedRemoteConnext[_origin]` doesn't return the zero address:\\n```\\ndiff // Remove the line below\\n// Remove the line below\\ngit a/derby// Remove the line below\\nyield// Remove the line below\\noptimiser/contracts/XProvider.sol b/derby// Remove the line below\\nyield// Remove the line below\\noptimiser/contracts/XProvider.sol\\nindex 6074fa0..f508a7c 100644\\n// Remove the line below\\n// Remove the line below\\n// Remove the line below\\n a/derby// Remove the line below\\nyield// Remove the line below\\noptimiser/contracts/XProvider.sol\\n// Add the line below\\n// Add the line below\\n// Add the line below\\n b/derby// Remove the line below\\nyield// Remove the line below\\noptimiser/contracts/XProvider.sol\\n@@ // Remove the line below\\n83,7 // Add the line below\\n83,7 @@ contract XProvider is IXReceiver {\\n    *    3) The call to this contract comes from Connext.\\n    */\\n   modifier onlySource(address _originSender, uint32 _origin) {\\n// Remove the line below\\n    require(_originSender == trustedRemoteConnext[_origin] && msg.sender == connext, "Not trusted");\\n// Add the line below\\n    require(trustedRemoteConnext[_origin] != address(0) && _originSender == trustedRemoteConnext[_origin] && msg.sender == connext, "Not trusted");\\n     _;\\n   }\\n```\\n
A malicious actor can call `XProvider.xReceive` and any functions of `XProvider` with the `onlySelf` modifier:\\nxReceive allow the caller to call any public function of `XProvider`, but only the ones with the `onlySelf` modifier are authorized;\\nreceiveAllocations can be used to corrupt allocations in the `XChainController` (i.e. allocate all tokens only to the protocol the attacker will benefit the most from);\\nreceiveTotalUnderlying can be used to set wrong "total underlying" value in the `XChainController` and block rebalancing of vaults (due to an underflow or another arithmetical error);\\nreceiveSetXChainAllocation can be used to set an exchange rate that will allow an attacker to drain a vault by redeeming their LP tokens at a higher rate;\\nreceiveFeedbackToXController can be used to trick the `XChainController` into skipping receiving of funds from a vault;\\nreceiveProtocolAllocationsToVault can be used by an attacker to unilaterally set allocations in a vault, directing funds only to protocol the attacker will benefit from;\\nreceiveRewardsToGame can be used by an attacker to increase the reward per LP token in a protocol the attacker deposited to;\\nfinally, receiveStateFeedbackToVault can allow an attacker to switch off a vault and exclude it from rebalancing.
```\\nmodifier onlySource(address _originSender, uint32 _origin) {\\n  require(_originSender == trustedRemoteConnext[_origin] && msg.sender == connext, "Not trusted");\\n  _;\\n}\\n```\\n