Fix audittens I11 (#45)

l1-contracts/contracts/bridge/asset-router/L1AssetRouter.sol

@@ -23,7 +23,7 @@ import {ReentrancyGuard} from "../../common/ReentrancyGuard.sol";
 import {DataEncoding} from "../../common/libraries/DataEncoding.sol";
 import {AddressAliasHelper} from "../../vendor/AddressAliasHelper.sol";
 import {TWO_BRIDGES_MAGIC_VALUE, ETH_TOKEN_ADDRESS} from "../../common/Config.sol";
-import {UnsupportedEncodingVersion, AssetIdNotSupported, AssetHandlerDoesNotExist, Unauthorized, ZeroAddress, TokenNotSupported, AddressAlreadyUsed} from "../../common/L1ContractErrors.sol";
+import {NonEmptyMsgValue, UnsupportedEncodingVersion, AssetIdNotSupported, AssetHandlerDoesNotExist, Unauthorized, ZeroAddress, TokenNotSupported, AddressAlreadyUsed} from "../../common/L1ContractErrors.sol";
 import {L2_ASSET_ROUTER_ADDR} from "../../common/L2ContractAddresses.sol";
 
 import {IBridgehub, L2TransactionRequestTwoBridgesInner, L2TransactionRequestDirect} from "../../bridgehub/IBridgehub.sol";
@@ -243,6 +243,10 @@ contract L1AssetRouter is AssetRouterBase, IL1AssetRouter, ReentrancyGuard {
         // The new encoding ensures that the calldata is collision-resistant with respect to the legacy format.
         // In the legacy calldata, the first input was the address, meaning the most significant byte was always `0x00`.
         if (encodingVersion == SET_ASSET_HANDLER_COUNTERPART_ENCODING_VERSION) {
+            if (msg.value != 0 || _value != 0) {
+                revert NonEmptyMsgValue();
+            }
+
             (bytes32 _assetId, address _assetHandlerAddressOnCounterpart) = abi.decode(_data[1:], (bytes32, address));
             return
                 _setAssetHandlerAddressOnCounterpart(

l1-contracts/contracts/bridge/interfaces/AssetHandlerModifiers.sol

@@ -0,0 +1,17 @@
+// SPDX-License-Identifier: MIT
+
+pragma solidity 0.8.24;
+
+import {NonEmptyMsgValue} from "../../common/L1ContractErrors.sol";
+
+abstract contract AssetHandlerModifiers {
+    /// @notice Modifier that ensures that a certain value is zero.
+    /// @dev This should be used in bridgeBurn-like functions to ensure that users
+    /// do not accidentally provide value there.
+    modifier requireZeroValue(uint256 _value) {
+        if (_value != 0) {
+            revert NonEmptyMsgValue();
+        }
+        _;
+    }
+}

l1-contracts/contracts/bridge/interfaces/IAssetHandler.sol

@@ -2,6 +2,8 @@
 
 pragma solidity 0.8.24;
 
+import {NonEmptyMsgValue} from "../../common/L1ContractErrors.sol";
+
 /// @title Asset Handler contract interface
 /// @author Matter Labs
 /// @custom:security-contact [email protected]

l1-contracts/contracts/bridge/ntv/L1NativeTokenVault.sol

@@ -24,6 +24,8 @@ import {ETH_TOKEN_ADDRESS} from "../../common/Config.sol";
 import {L2_NATIVE_TOKEN_VAULT_ADDR} from "../../common/L2ContractAddresses.sol";
 import {DataEncoding} from "../../common/libraries/DataEncoding.sol";
 
+import {AssetHandlerModifiers} from "../interfaces/AssetHandlerModifiers.sol";
+
 import {Unauthorized, ZeroAddress, NoFundsTransferred, InsufficientChainBalance, WithdrawFailed, OriginChainIdNotFound} from "../../common/L1ContractErrors.sol";
 
 /// @author Matter Labs
@@ -193,7 +195,7 @@ contract L1NativeTokenVault is IL1NativeTokenVault, IL1AssetHandler, NativeToken
         bytes32 _assetId,
         address _depositSender,
         bytes calldata _data
-    ) external payable override onlyAssetRouter whenNotPaused {
+    ) external payable override requireZeroValue(msg.value) onlyAssetRouter whenNotPaused {
         (uint256 _amount, ) = abi.decode(_data, (uint256, address));
         address l1Token = tokenAddress[_assetId];
         if (_amount == 0) {

l1-contracts/contracts/bridge/ntv/NativeTokenVault.sol

@@ -22,12 +22,19 @@ import {BridgedStandardERC20} from "../BridgedStandardERC20.sol";
 import {BridgeHelper} from "../BridgeHelper.sol";
 
 import {AssetIdAlreadyRegistered, EmptyDeposit, Unauthorized, TokensWithFeesNotSupported, TokenNotSupported, NonEmptyMsgValue, ValueMismatch, AddressMismatch, AssetIdMismatch, AmountMustBeGreaterThanZero, ZeroAddress, DeployingBridgedTokenForNativeToken} from "../../common/L1ContractErrors.sol";
+import {AssetHandlerModifiers} from "../interfaces/AssetHandlerModifiers.sol";
 
 /// @author Matter Labs
 /// @custom:security-contact [email protected]
 /// @dev Vault holding L1 native ETH and ERC20 tokens bridged into the ZK chains.
 /// @dev Designed for use with a proxy for upgradability.
-abstract contract NativeTokenVault is INativeTokenVault, IAssetHandler, Ownable2StepUpgradeable, PausableUpgradeable {
+abstract contract NativeTokenVault is
+    INativeTokenVault,
+    IAssetHandler,
+    Ownable2StepUpgradeable,
+    PausableUpgradeable,
+    AssetHandlerModifiers
+{
     using SafeERC20 for IERC20;
 
     /// @dev The address of the WETH token.
@@ -118,7 +125,7 @@ abstract contract NativeTokenVault is INativeTokenVault, IAssetHandler, Ownable2
         uint256 _chainId,
         bytes32 _assetId,
         bytes calldata _data
-    ) external payable override onlyAssetRouter whenNotPaused {
+    ) external payable override requireZeroValue(msg.value) onlyAssetRouter whenNotPaused {
         address receiver;
         uint256 amount;
         // we set all originChainId for all already bridged tokens with the setLegacyTokenAssetId and updateChainBalancesFromSharedBridge functions.
@@ -175,11 +182,19 @@ abstract contract NativeTokenVault is INativeTokenVault, IAssetHandler, Ownable2
     /// @dev In case of native token vault _data is the tuple of _depositAmount and _receiver.
     function bridgeBurn(
         uint256 _chainId,
-        uint256,
+        uint256 _l2MsgValue,
         bytes32 _assetId,
         address _originalCaller,
         bytes calldata _data
-    ) external payable override onlyAssetRouter whenNotPaused returns (bytes memory _bridgeMintData) {
+    )
+        external
+        payable
+        override
+        requireZeroValue(_l2MsgValue)
+        onlyAssetRouter
+        whenNotPaused
+        returns (bytes memory _bridgeMintData)
+    {
         if (originChainId[_assetId] != block.chainid) {
             _bridgeMintData = _bridgeBurnBridgedToken(_chainId, _assetId, _originalCaller, _data);
         } else {
@@ -198,7 +213,7 @@ abstract contract NativeTokenVault is INativeTokenVault, IAssetHandler, Ownable2
         bytes32 _assetId,
         address _originalCaller,
         bytes calldata _data
-    ) internal returns (bytes memory _bridgeMintData) {
+    ) internal requireZeroValue(msg.value) returns (bytes memory _bridgeMintData) {
         (uint256 _amount, address _receiver) = abi.decode(_data, (uint256, address));
         if (_amount == 0) {
             // "Amount cannot be zero");

l1-contracts/contracts/bridgehub/Bridgehub.sol

@@ -25,14 +25,16 @@ import {IMessageRoot} from "./IMessageRoot.sol";
 import {ICTMDeploymentTracker} from "./ICTMDeploymentTracker.sol";
 import {MigrationPaused, AssetIdAlreadyRegistered, ChainAlreadyLive, ChainNotLegacy, CTMNotRegistered, ChainIdNotRegistered, AssetHandlerNotRegistered, ZKChainLimitReached, CTMAlreadyRegistered, CTMNotRegistered, ZeroChainId, ChainIdTooBig, BridgeHubAlreadyRegistered, AddressTooLow, MsgValueMismatch, ZeroAddress, Unauthorized, SharedBridgeNotSet, WrongMagicValue, ChainIdAlreadyExists, ChainIdMismatch, ChainIdCantBeCurrentChain, EmptyAssetId, AssetIdNotSupported, IncorrectBridgeHubAddress} from "../common/L1ContractErrors.sol";
 
+import {AssetHandlerModifiers} from "../bridge/interfaces/AssetHandlerModifiers.sol";
+
 /// @author Matter Labs
 /// @custom:security-contact [email protected]
 /// @dev The Bridgehub contract serves as the primary entry point for L1<->L2 communication,
 /// facilitating interactions between end user and bridges.
 /// It also manages state transition managers, base tokens, and chain registrations.
 /// Bridgehub is also an IL1AssetHandler for the chains themselves, which is used to migrate the chains
 /// between different settlement layers (for example from L1 to Gateway).
-contract Bridgehub is IBridgehub, ReentrancyGuard, Ownable2StepUpgradeable, PausableUpgradeable {
+contract Bridgehub is IBridgehub, ReentrancyGuard, Ownable2StepUpgradeable, PausableUpgradeable, AssetHandlerModifiers {
     using EnumerableMap for EnumerableMap.UintToAddressMap;
 
     /// @notice the asset id of Eth. This is only used on L1.
@@ -689,11 +691,19 @@ contract Bridgehub is IBridgehub, ReentrancyGuard, Ownable2StepUpgradeable, Paus
     /// @param _data the data for the migration
     function bridgeBurn(
         uint256 _settlementChainId,
-        uint256, // msgValue
+        uint256 _l2MsgValue,
         bytes32 _assetId,
         address _originalCaller,
         bytes calldata _data
-    ) external payable override onlyAssetRouter whenMigrationsNotPaused returns (bytes memory bridgehubMintData) {
+    )
+        external
+        payable
+        override
+        requireZeroValue(_l2MsgValue + msg.value)
+        onlyAssetRouter
+        whenMigrationsNotPaused
+        returns (bytes memory bridgehubMintData)
+    {
         require(whitelistedSettlementLayers[_settlementChainId], "BH: SL not whitelisted");
 
         BridgehubBurnCTMAssetData memory bridgehubData = abi.decode(_data, (BridgehubBurnCTMAssetData));
@@ -734,7 +744,7 @@ contract Bridgehub is IBridgehub, ReentrancyGuard, Ownable2StepUpgradeable, Paus
         uint256, // originChainId
         bytes32 _assetId,
         bytes calldata _bridgehubMintData
-    ) external payable override onlyAssetRouter whenMigrationsNotPaused {
+    ) external payable override requireZeroValue(msg.value) onlyAssetRouter whenMigrationsNotPaused {
         BridgehubMintCTMAssetData memory bridgehubData = abi.decode(_bridgehubMintData, (BridgehubMintCTMAssetData));
 
         address ctm = ctmAssetIdToAddress[_assetId];
@@ -771,7 +781,7 @@ contract Bridgehub is IBridgehub, ReentrancyGuard, Ownable2StepUpgradeable, Paus
         bytes32 _assetId,
         address _depositSender,
         bytes calldata _data
-    ) external payable override onlyAssetRouter onlyL1 {
+    ) external payable override requireZeroValue(msg.value) onlyAssetRouter onlyL1 {
         BridgehubBurnCTMAssetData memory bridgehubData = abi.decode(_data, (BridgehubBurnCTMAssetData));
 
         settlementLayer[bridgehubData.chainId] = block.chainid;

l1-contracts/test/foundry/l1/unit/concrete/Bridgehub/experimental_bridge.t.sol

@@ -1402,7 +1402,6 @@ contract ExperimentalBridgeTest is Test {
         uint256 chainId,
         uint256 mintValue,
         uint256 msgValue,
-        uint256 l2Value,
         uint256 l2GasLimit,
         uint256 l2GasPerPubdataByteLimit,
         address refundRecipient,
@@ -1422,7 +1421,7 @@ contract ExperimentalBridgeTest is Test {
         L2TransactionRequestTwoBridgesOuter memory l2TxnReq2BridgeOut = _createMockL2TransactionRequestTwoBridgesOuter({
             chainId: chainId,
             mintValue: mintValue,
-            l2Value: l2Value,
+            l2Value: 0,
             l2GasLimit: l2GasLimit,
             l2GasPerPubdataByteLimit: l2GasPerPubdataByteLimit,
             refundRecipient: refundRecipient,

l1-contracts/test/foundry/l1/unit/concrete/Bridges/L1SharedBridge/L1SharedBridgeHyperEnabled.t.sol

@@ -60,7 +60,7 @@ contract L1AssetRouterHyperEnabledTest is L1AssetRouterTest {
         sharedBridge.bridgehubDeposit{value: amount}({
             _chainId: chainId,
             _originalCaller: alice,
-            _value: amount,
+            _value: 0,
             _data: abi.encode(ETH_TOKEN_ADDRESS, amount, bob)
         });
     }