Fix audittens H02 (#16)

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

@@ -507,9 +507,8 @@ contract L1AssetRouter is AssetRouterBase, IL1AssetRouter, ReentrancyGuard {
         }
 
         bytes32 _assetId;
-        bytes memory bridgeMintCalldata;
-
         {
+            bytes memory bridgeMintCalldata;
             // Inner call to encode data to decrease local var numbers
             _assetId = _ensureTokenRegisteredWithNTV(_l1Token);
             IERC20(_l1Token).forceApprove(address(nativeTokenVault), _amount);
@@ -522,9 +521,7 @@ contract L1AssetRouter is AssetRouterBase, IL1AssetRouter, ReentrancyGuard {
                 _transferData: abi.encode(_amount, _l2Receiver),
                 _passValue: false
             });
-        }
 
-        {
             bytes memory l2TxCalldata = getDepositCalldata(_originalCaller, _assetId, bridgeMintCalldata);
 
             // If the refund recipient is not specified, the refund will be sent to the sender of the transaction.
@@ -546,12 +543,21 @@ contract L1AssetRouter is AssetRouterBase, IL1AssetRouter, ReentrancyGuard {
             txHash = BRIDGE_HUB.requestL2TransactionDirect{value: msg.value}(request);
         }
 
-        // Save the deposited amount to claim funds on L1 if the deposit failed on L2
-        L1_NULLIFIER.bridgehubConfirmL2TransactionForwarded(
-            ERA_CHAIN_ID,
-            DataEncoding.encodeLegacyTxDataHash(_originalCaller, _l1Token, _amount),
-            txHash
-        );
+        {
+            bytes memory transferData = abi.encode(_amount, _l2Receiver);
+            // Save the deposited amount to claim funds on L1 if the deposit failed on L2
+            L1_NULLIFIER.bridgehubConfirmL2TransactionForwarded(
+                ERA_CHAIN_ID,
+                DataEncoding.encodeTxDataHash({
+                    _encodingVersion: LEGACY_ENCODING_VERSION,
+                    _originalCaller: _originalCaller,
+                    _assetId: _assetId,
+                    _nativeTokenVault: address(nativeTokenVault),
+                    _transferData: transferData
+                }),
+                txHash
+            );
+        }
 
         emit LegacyDepositInitiated({
             chainId: ERA_CHAIN_ID,

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

@@ -21,7 +21,7 @@ import {DataEncoding} from "../../common/libraries/DataEncoding.sol";
 import {BridgedStandardERC20} from "../BridgedStandardERC20.sol";
 import {BridgeHelper} from "../BridgeHelper.sol";
 
-import {EmptyDeposit, Unauthorized, TokensWithFeesNotSupported, TokenNotSupported, NonEmptyMsgValue, ValueMismatch, AddressMismatch, AssetIdMismatch, AmountMustBeGreaterThanZero, ZeroAddress, DeployingBridgedTokenForNativeToken} from "../../common/L1ContractErrors.sol";
+import {AssetIdAlreadyRegistered, EmptyDeposit, Unauthorized, TokensWithFeesNotSupported, TokenNotSupported, NonEmptyMsgValue, ValueMismatch, AddressMismatch, AssetIdMismatch, AmountMustBeGreaterThanZero, ZeroAddress, DeployingBridgedTokenForNativeToken} from "../../common/L1ContractErrors.sol";
 
 /// @author Matter Labs
 /// @custom:security-contact [email protected]
@@ -92,6 +92,9 @@ abstract contract NativeTokenVault is INativeTokenVault, IAssetHandler, Ownable2
             revert TokenNotSupported(WETH_TOKEN);
         }
         require(_nativeToken.code.length > 0, "NTV: empty token");
+        if (assetId[_nativeToken] != bytes32(0)) {
+            revert AssetIdAlreadyRegistered();
+        }
         _unsafeRegisterNativeToken(_nativeToken);
     }
 

l1-contracts/contracts/common/L1ContractErrors.sol

@@ -416,6 +416,8 @@ error NotBridgehub(address addr);
 error InvalidAddress(address expected, address actual);
 // 0xfa5cd00f
 error NotAllowed(address addr);
+// 0x1929b7de
+error IncorrectTokenAddressFromNTV(bytes32 assetId, address tokenAddress);
 
 enum SharedBridgeKey {
     PostUpgradeFirstBatch,

l1-contracts/contracts/common/libraries/DataEncoding.sol

@@ -5,7 +5,7 @@ pragma solidity 0.8.24;
 import {L2_NATIVE_TOKEN_VAULT_ADDR} from "../L2ContractAddresses.sol";
 import {LEGACY_ENCODING_VERSION, NEW_ENCODING_VERSION} from "../../bridge/asset-router/IAssetRouterBase.sol";
 import {INativeTokenVault} from "../../bridge/ntv/INativeTokenVault.sol";
-import {UnsupportedEncodingVersion} from "../L1ContractErrors.sol";
+import {IncorrectTokenAddressFromNTV, UnsupportedEncodingVersion} from "../L1ContractErrors.sol";
 
 /**
  * @author Matter Labs
@@ -107,8 +107,17 @@ library DataEncoding {
     ) internal view returns (bytes32 txDataHash) {
         if (_encodingVersion == LEGACY_ENCODING_VERSION) {
             address tokenAddress = INativeTokenVault(_nativeTokenVault).tokenAddress(_assetId);
+
+            // This is a double check to ensure that the used token for the legacy encoding is correct.
+            // This revert should never be emitted and in real life and should only serve as a guard in
+            // case of inconsistent state of Native Token Vault.
+            bytes32 expectedAssetId = encodeNTVAssetId(block.chainid, tokenAddress);
+            if (_assetId != expectedAssetId) {
+                revert IncorrectTokenAddressFromNTV(_assetId, tokenAddress);
+            }
+
             (uint256 depositAmount, ) = abi.decode(_transferData, (uint256, address));
-            txDataHash = encodeLegacyTxDataHash(_originalCaller, tokenAddress, depositAmount);
+            txDataHash = keccak256(abi.encode(_originalCaller, tokenAddress, depositAmount));
         } else if (_encodingVersion == NEW_ENCODING_VERSION) {
             // Similarly to calldata, the txDataHash is collision-resistant.
             // In the legacy data hash, the first encoded variable was the address, which is padded with zeros during `abi.encode`.
@@ -120,20 +129,6 @@ library DataEncoding {
         }
     }
 
-    /// @notice Encodes the legacy transaction data hash.
-    /// @dev the encoding strats with 0t
-    /// @param _originalCaller The address of the entity that initiated the deposit.
-    /// @param _l1Token The address of the L1 token.
-    /// @param _amount The amount of the L1 token.
-    /// @return txDataHash The resulting encoded transaction data hash.
-    function encodeLegacyTxDataHash(
-        address _originalCaller,
-        address _l1Token,
-        uint256 _amount
-    ) internal pure returns (bytes32) {
-        return keccak256(abi.encode(_originalCaller, _l1Token, _amount));
-    }
-
     /// @notice Decodes the token data by combining chain id, asset deployment tracker and asset data.
     function decodeTokenData(
         bytes calldata _tokenData

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

@@ -107,7 +107,7 @@ contract L1AssetRouterTestBase is L1AssetRouterTest {
         vm.mockCall(
             address(nativeTokenVault),
             abi.encodeWithSelector(IL1BaseTokenAssetHandler.tokenAddress.selector, tokenAssetId),
-            abi.encode(address(0))
+            abi.encode(address(token))
         );
         vm.prank(bridgehubAddress);
         sharedBridge.bridgehubDeposit(chainId, alice, 0, abi.encode(address(token), amount, bob));

l1-contracts/test/foundry/l1/unit/concrete/Bridges/NativeTokenVault/L1NativeTokenVault.sol

@@ -0,0 +1,47 @@
+import {StdStorage, stdStorage} from "forge-std/Test.sol";
+import {Test} from "forge-std/Test.sol";
+import {Vm} from "forge-std/Vm.sol";
+
+import {L1NativeTokenVault} from "contracts/bridge/ntv/L1NativeTokenVault.sol";
+import {L1AssetRouter} from "contracts/bridge/asset-router/L1AssetRouter.sol";
+import {IL1Nullifier} from "contracts/bridge/interfaces/IL1Nullifier.sol";
+import {AssetIdAlreadyRegistered} from "contracts/common/L1ContractErrors.sol";
+
+contract SomeToken {
+    constructor() {}
+
+    function name() external {
+        // Just some function so that the bytecode is not empty,
+        // the actional functionality is not used.
+    }
+}
+
+contract L1NativeTokenVaultTest is Test {
+    address assetRouter;
+
+    L1NativeTokenVault ntv;
+    SomeToken token;
+
+    function setUp() public {
+        assetRouter = makeAddr("assetRouter");
+
+        ntv = new L1NativeTokenVault(makeAddr("wethToken"), assetRouter, uint256(0), IL1Nullifier(address(0)));
+
+        token = new SomeToken();
+    }
+
+    function test_revertWhenRegisteringSameAddressTwice() external {
+        vm.mockCall(
+            assetRouter,
+            abi.encodeCall(
+                L1AssetRouter.setAssetHandlerAddressThisChain,
+                (bytes32(uint256(uint160(address(token)))), address(ntv))
+            ),
+            hex""
+        );
+        ntv.registerToken(address(token));
+
+        vm.expectRevert(AssetIdAlreadyRegistered.selector);
+        ntv.registerToken(address(token));
+    }
+}