diff --git a/packages/protocol/src/routers/ConnextRouter.sol b/packages/protocol/src/routers/ConnextRouter.sol index f442ef98e..c044d572a 100644 --- a/packages/protocol/src/routers/ConnextRouter.sol +++ b/packages/protocol/src/routers/ConnextRouter.sol @@ -31,6 +31,13 @@ contract ConnextRouter is BaseRouter, IXReceiver { */ event NewRouterAdded(address indexed router, uint256 indexed domain); + /** + * @dev Emitted when a new _catchGasCost value is set. + * + * @param newCatchGasCost value + */ + event CatchGasCostChanged(uint256 newCatchGasCost); + /** * @dev Emitted when Connext `xCall` is invoked. * @@ -74,9 +81,11 @@ contract ConnextRouter is BaseRouter, IXReceiver { /// @dev Custom Errors error ConnextRouter__setRouter_invalidInput(); error ConnextRouter__xReceive_notReceivedAssetBalance(); - error ConnextRouter__xReceive_notAllowedCaller(); - error ConnextRouter__xReceiver_noValueTransferUseXbundle(); + error ConnnextRouter__xReceive_notEnoughGasForTryCatch(); error ConnnextRouter__xBundleConnext_notSelfCalled(); + error ConnextRouter__setCatchExecutionGasCost_zeroValue(); + + uint256 private _catchGasCost; /// @dev The connext contract on the origin domain. IConnext public immutable connext; @@ -102,6 +111,7 @@ contract ConnextRouter is BaseRouter, IXReceiver { connext = connext_; handler = new ConnextHandler(address(this)); _allowCaller(msg.sender, true); + _catchGasCost = 670000; } /*//////////////////////////////////// @@ -168,7 +178,7 @@ contract ConnextRouter is BaseRouter, IXReceiver { * to `xReceive` fails. That's why we need to ensure the funds are not stuck at Connext. * Therefore we try/catch instead of directly calling _bundleInternal(...). */ - try this.xBundleConnext(actions, args, beforeSlipped) { + try this.xBundleConnext{gas: gasleft() - _catchGasCost}(actions, args, beforeSlipped) { emit XReceived(transferId, originDomain, true, asset, amount, callData); } catch { if (balance > 0) { @@ -465,4 +475,23 @@ contract ConnextRouter is BaseRouter, IXReceiver { emit NewRouterAdded(router, domain); } + + /** + * @notice Sets a new value for `_catchGasCost`. + * + * @param newCatchGasCost value + * + * @dev Requirements: + * - Must be restricted to timelock. + * - Must ensure through testing that `newCatchGasCost` > than most + * gas heavy call in {ConnextHandler-recordFailed(...)}. + */ + function setCatchGasExecutionCost(uint256 newCatchGasCost) external onlyTimelock { + if (newCatchGasCost == 0) { + revert ConnextRouter__setCatchExecutionGasCost_zeroValue(); + } + _catchGasCost = newCatchGasCost; + + emit CatchGasCostChanged(newCatchGasCost); + } } diff --git a/packages/protocol/test/forking/goerli/ConnextRouter.t.sol b/packages/protocol/test/forking/goerli/ConnextRouter.t.sol index 26f3833df..5296f8eab 100644 --- a/packages/protocol/test/forking/goerli/ConnextRouter.t.sol +++ b/packages/protocol/test/forking/goerli/ConnextRouter.t.sol @@ -58,6 +58,12 @@ contract MockTestFlasher is Routines, IFlasher { } } +contract MaliciousReceiver { + receive() external payable { + while (true) {} + } +} + contract ConnextRouterForkingTests is Routines, ForkingSetup { event Deposit(address indexed sender, address indexed owner, uint256 assets, uint256 shares); @@ -265,6 +271,35 @@ contract ConnextRouterForkingTests is Routines, ForkingSetup { assertEq(IERC20(collateralAsset).balanceOf(address(connextHandler)), amount); } + function test_gasGriefingInboundXBundle() public { + MaliciousReceiver maliciousReceiver = new MaliciousReceiver(); + + uint256 amount = 1 ether; + + // make the callData with malicious receiver + IRouter.Action[] memory actions_ = new IRouter.Action[](1); + actions_[0] = IRouter.Action.WithdrawETH; + + bytes[] memory args_ = new bytes[](1); + args_[0] = abi.encode(amount, address(maliciousReceiver)); + + bytes memory callData = abi.encode(actions_, args_); + + // send directly the bridged funds to our router + // thus mocking Connext behavior + deal(collateralAsset, address(connextRouter), amount); + + vm.startPrank(registry[domain].connext); + // Limit the amount of gas being passed + connextRouter.xReceive{gas: 750000}( + "", amount, vault.asset(), address(connextRouter), OPTIMISM_GOERLI_DOMAIN, callData + ); + vm.stopPrank(); + + // Funds were moved to ConnextHandler regardles of maliciousReceiver + assertEq(IERC20(collateralAsset).balanceOf(address(connextHandler)), amount); + } + function test_retryFailedInboundXReceive() public { uint256 amount = 2 ether; uint256 borrowAmount = 1000e6;