zkSync v2 contest - ctf_sec's results

Lines of code

https://github.com/code-423n4/2022-10-zksync/blob/456078b53a6d09636b84522ac8f3e8049e4e3af5/ethereum/contracts/bridge/L1EthBridge.sol#L96

Vulnerability details

Impact

When user deposit in L1ETHBridge.sol, they can avoid paying the fee by making msg.value == amount

Proof of Concept

Let us check this code in L1ETHBridge::deposit

// Will revert if msg.value is less than the amount of the deposit
uint256 zkSyncFee = msg.value - _amount;
bytes memory l2TxCalldata = _getDepositL2Calldata(msg.sender, _l2Receiver, _amount);
txHash = zkSyncMailbox.requestL2Transaction{value: zkSyncFee}(
	l2Bridge,
	0, // L2 msg.value
	l2TxCalldata,
	DEPOSIT_ERGS_LIMIT,
	new bytes[](0)
);

// Save the deposit amount, to claim funds back if the L2 transaction will failed
depositAmount[msg.sender][txHash] = _amount;

For example, the user can setting the msg.value to 1 ETH and amount to 1 ETH.

msg.value - amount is 0 so the user does not need to pay the zkSyncFee,

however, note the line

// Save the deposit amount, to claim funds back if the L2 transaction will failed
depositAmount[msg.sender][txHash] = _amount;

the amount 1 ETH is still recorded in the smart contract!

Tools Used

Code Insepection

Recommended Mitigation Steps

The dev can change the code to enforce the logic that % of fee needs to be charged before calling txHash = zkSyncMailbox.requestL2Transaction{value: zkSyncFee}