Users will lose all ETH sent as `cost` parameter in transactions to and from Optimism

mediumCode4rena

Lines of code

https://github.com/code-423n4/2024-05-olas/blob/main/tokenomics/contracts/staking/OptimismDepositProcessorL1.sol#L140 https://github.com/code-423n4/2024-05-olas/blob/main/tokenomics/contracts/staking/OptimismDepositProcessorL1.sol#L121

Vulnerability details

The OptimismDepositProcessorL1 contract is designed to send tokens and data from L1 to L2 using the Optimism bridge. It interacts with the L1CrossDomainMessenger to deposit ERC20 tokens and send messages.

The _sendMessage() function extracts the cost and gasLimitMessage variables from the bridgePayload function parameter, which is user-provided and passed on from the Dispenser contract.

The cost variable is meant to represent the costs the user must pay to the bridge for sending and delivering the message and is verified to be within 0 < cost <= msg.value before being passed on as value to L1CrossDomainMessenger.sendMessage(). However, this is not how Optimism charges for fees, and the ETH sent with this transaction is unnecessary and will lead to loss for the caller.

According to the Optimism documentation, the cost of message delivery is covered by burning gas on L1, not by transferring ETH to the bridging functions. As can be seen in the sendMessage() implementation, this value is encoded as parameter to relayMessage() on the receiving side:

solidity
File: CrossDomainMessenger.sol
176:     function sendMessage(address _target, bytes calldata _message, uint32 _minGasLimit) external payable {
177:         if (isCustomGasToken()) {
178:             require(msg.value == 0, "CrossDomainMessenger: cannot send value with custom gas token");
179:         }
180: 
181:         // Triggers a message to the other messenger. Note that the amount of gas provided to the
182:         // message is the amount of gas requested by the user PLUS the base gas value. We want to
183:         // guarantee the property that the call to the target contract will always have at least
184:         // the minimum gas limit specified by the user.
185:         _sendMessage({
186:             _to: address(otherMessenger),
187:             _gasLimit: baseGas(_message, _minGasLimit),
188:             _value: msg.value,
189:             _data: abi.encodeWithSelector(
190:                 this.relayMessage.selector, messageNonce(), msg.sender, _target, msg.value, _minGasLimit, _message
191:             )
192:         });

And it is then passed in full as msg.value along with the message:

solidity
File: CrossDomainMessenger.sol
211:     function relayMessage(
212:         uint256 _nonce,
213:         address _sender,
214:         address _target,
215:         uint256 _value,
216:         uint256 _minGasLimit,
217:         bytes calldata _message
218:     )
219:         external
220:         payable
221:     {

			 ...

287:         bool success = SafeCall.call(_target, gasleft() - RELAY_RESERVED_GAS, _value, _message);

Therefore, passing cost as msg.value results in the entire amount being sent to the receiveMessage() function on L2, where it remains unused in the OptimismTargetDispenserL2 contract. This behavior forces users to pay ETH that serves no purpose.

The OptimismTargetDispenserL2 implements the same behaviour in _sendMessage(). However, this function is only reachable from syncWithheldTokens(), which will likely not usually be called by users.

Impact

Users are forced to pay ETH (which would likely be calculated as some multiple of gasLimitMessage) that is not utilized for its intended purpose, resulting in a direct loss.
The OptimismTargetDispenserL2 contract accumulates unnecessary ETH.

Proof of Concept

A user initiates a token transfer from L1 to Optimism through the Dispenser contract, which calls the OptimismDepositProcessorL1 contract.
The _sendMessage() function is called with a cost parameter.
The cost is passed as msg.value to the sendMessage() function of Optimism's L1CrossDomainMessenger contract.
The sendMessage() function sends the msg.value to the receiveMessage() function on L2.
The ETH remains in the OptimismTargetDispenserL2 contract, unused, resulting in a loss for the user.

Tools Used

Manual review

Recommended Mitigation Steps

Modify the _sendMessage() function to only decode a gasLimitMessage variable from the bridgePayload parameter, and do not pass any ETH to L1CrossDomainMessenger.sendMessage(). The user should be responsible for submitting the transaction with a high enough gas limit to send the message.