MorpheusAI
MorpheusAI
Foundry
22,500 USDC
View results
Previous Next
Submission Details
Severity: medium
Invalid

GatewayRouterMock.outboundTransfer() only implementing transferFrom to transfer token from L1 to L2 without implementing real deployment implementation based on Arbitrum

0xdemon

Summary

In real deployment GatewayRouter.outboundTransfer() has 3 important variables to determine {value : msg.value} as gas for execution on the Arbitrum chain. The 3 variables are gasLimit_, maxFeePerGas_, and maxSubmissionCost_. However, the {value : msg.value} calculation is not applied to GatewayRouterMock.outboundTransfer() so whatever the values of gasLimit_, maxFeePerGas_, and maxSubmissionCost_, the test results will be successful ( even {value : 0} ) because the function only transfers tokens with transferFrom.

Vulnerability Details

This is the GatewayRouterMock.outboundTransfer() codebase :

File : contracts/mock/GatewayRouterMock.sol
​
function outboundTransfer(
address _token,
address _to,
uint256 _amount,
uint256 _maxGas, // gasLimit_
uint256 _gasPriceBid, // maxFeePerGas_
bytes calldata _data
) external payable returns (bytes memory) {
IERC20(_token).transferFrom(msg.sender, _to, _amount);
​
return abi.encode(_token, _to, _amount, _maxGas, _gasPriceBid, _data);
}

Based on this codebase, whatever input for gasLimit, maxFeePerGas, maxSubmissionCost and call value {value : msg.value} the test results will be successful.
Even though based on docs and tests from Arbitrum, call value must pass maxSubmissionCost + maxGas * gasPriceBid. Funds can be lost if the L1 call value provided is insufficient to cover maxSubmissionCost, or stuck if insufficient to cover maxSubmissionCost + maxGas * gasPriceBid.

POC

Test code variables :

gasLimit = 10000000000

maxFeePerGas =15000000000

maxSubmissionCost = 1000000000

{value : msg.value} = 0

it('should success whatever the value of gasLimit_, maxFeePerGas_, maxSubmissionCost_, msg.value', async () => {
const l2TokenReceiverAddress = await l2TokenReceiver.getAddress();
​
await distribution.stake(1, wei(1));
​
await depositToken.setTotalPooledEther((await depositToken.totalPooledEther()) * 2n);
​
const overplus = await distribution.overplus();
expect(overplus).to.eq(wei(1));
​
const bridgeMessageId = await distribution.bridgeOverplus.staticCall(10000000000, 15000000000, 1000000000, {value : 0});
const tx = await distribution.bridgeOverplus(10000000000, 15000000000, 1000000000, {value : 0});
await expect(tx).to.emit(distribution, 'OverplusBridged').withArgs(wei(1), bridgeMessageId);
await expect(tx).to.changeTokenBalance(depositToken, distribution, wei(-1));
expect(await wstETH.balanceOf(l2TokenReceiverAddress)).to.eq(wei(1));
});

Result :

Distribution
#bridgeOverplus
✔ should success whatever the value of gasLimit_, maxFeePerGas_, maxSubmissionCost_, msg.value (161ms)
​
1 passing (3s)

This coded POC was written using the default environment of the protocol so just input in the Distribution.test.ts file and paste it in the bridgeOverplus section then the POC can be executed immediately.

Impact

Wrong result in all tests (related to this function) and lead to vulnerability on deployment (mainnet)

Tools Used

Manual Review

Recommended Mitigation

Consider implementing the real deployment implementation based on Arbitrum docs and test :

File : contracts/mock/GatewayRouterMock.sol
​
function outboundTransfer(
address _token,
address _to,
uint256 _amount,
uint256 _maxGas, // gasLimit_
uint256 _gasPriceBid, // maxFeePerGas_
bytes calldata _data
) external payable returns (bytes memory) {
uint256 _maxSubmissionCost = abi.decode(_data, (uint256));
uint256 expectedEth = _maxSubmissionCost + (_maxGas * _gasPriceBid);
require(_maxSubmissionCost > 0, "NO_SUBMISSION_COST");
require(msg.value == expectedEth, "WRONG_ETH_VALUE");
​
IERC20(_token).transferFrom(msg.sender, _to, _amount);
​
return abi.encode(_token, _to, _amount, _maxGas, _gasPriceBid, _data);
}
Updates

Lead Judging Commences

inallhonesty Lead Judge over 1 year ago
Submission Judgement Published
Validated
Assigned finding tags:

GatewayRouterMock lacks some functionality

inallhonesty Lead Judge over 1 year ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.