Fix audittens m02 (#21)

matter-labs · Dec 2, 2024 · 59d11f7 · 59d11f7
1 parent 2bcd5de
commit 59d11f7
Show file tree

Hide file tree

Showing 6 changed files with 16 additions and 11 deletions.
diff --git a/l1-contracts/contracts/bridge/L1ERC20Bridge.sol b/l1-contracts/contracts/bridge/L1ERC20Bridge.sol
@@ -13,7 +13,7 @@ import {IL1AssetRouter} from "./asset-router/IL1AssetRouter.sol";
 import {L2ContractHelper} from "../common/libraries/L2ContractHelper.sol";
 import {ReentrancyGuard} from "../common/ReentrancyGuard.sol";
 
-import {EmptyDeposit, WithdrawalAlreadyFinalized, TokensWithFeesNotSupported, ETHDepositNotSupported, ApprovalFailed} from "../common/L1ContractErrors.sol";
+import {AssetRouterAllowanceNotZero, EmptyDeposit, WithdrawalAlreadyFinalized, TokensWithFeesNotSupported, ETHDepositNotSupported} from "../common/L1ContractErrors.sol";
 import {ETH_TOKEN_ADDRESS} from "../common/Config.sol";
 
 /// @author Matter Labs
@@ -203,10 +203,9 @@ contract L1ERC20Bridge is IL1ERC20Bridge, ReentrancyGuard {
             _l2TxGasPerPubdataByte: _l2TxGasPerPubdataByte,
             _refundRecipient: _refundRecipient
         });
-        // clearing approval
-        bool success = IERC20(_l1Token).approve(address(L1_ASSET_ROUTER), 0);
-        if (!success) {
-            revert ApprovalFailed();
+        // Ensuring that all the funds that were locked into this bridge were spent by the asset router / native token vault.
+        if (IERC20(_l1Token).allowance(address(this), address(L1_ASSET_ROUTER)) != 0) {
+            revert AssetRouterAllowanceNotZero();
         }
         depositAmount[msg.sender][_l1Token][l2TxHash] = _amount;
         emit DepositInitiated({

diff --git a/l1-contracts/contracts/bridge/asset-router/L1AssetRouter.sol b/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 {NonEmptyMsgValue, UnsupportedEncodingVersion, AssetIdNotSupported, AssetHandlerDoesNotExist, Unauthorized, ZeroAddress, TokenNotSupported, AddressAlreadyUsed} from "../../common/L1ContractErrors.sol";
+import {LegacyBridgeUsesNonNativeToken, 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";
@@ -529,6 +529,13 @@ contract L1AssetRouter is AssetRouterBase, IL1AssetRouter, ReentrancyGuard {
             bytes memory bridgeMintCalldata;
             // Inner call to encode data to decrease local var numbers
             _assetId = _ensureTokenRegisteredWithNTV(_l1Token);
+            // Legacy bridge is only expected to use native tokens for L1.
+            if (_assetId != DataEncoding.encodeNTVAssetId(block.chainid, _l1Token)) {
+                revert LegacyBridgeUsesNonNativeToken();
+            }
+
+            IERC20(_l1Token).forceApprove(address(nativeTokenVault), _amount);
+
             bridgeMintCalldata = _burn({
                 _chainId: ERA_CHAIN_ID,
                 _nextMsgValue: 0,

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

diff --git a/l1-contracts/contracts/bridge/ntv/L1NativeTokenVault.sol b/l1-contracts/contracts/bridge/ntv/L1NativeTokenVault.sol
@@ -24,8 +24,6 @@ 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

diff --git a/l1-contracts/contracts/common/L1ContractErrors.sol b/l1-contracts/contracts/common/L1ContractErrors.sol
@@ -426,6 +426,10 @@ error InvalidProofLengthForFinalNode();
 error TokenIsNotLegacy();
 // 0xa51fa558
 error TokenIsLegacy();
+// 0x29963361
+error LegacyBridgeUsesNonNativeToken();
+// 0x11832de8
+error AssetRouterAllowanceNotZero();
 // 0xb20b58ce
 error NoLegacySharedBridge();
 

diff --git a/l1-contracts/contracts/state-transition/chain-deps/facets/Getters.sol b/l1-contracts/contracts/state-transition/chain-deps/facets/Getters.sol
@@ -14,7 +14,6 @@ import {IBridgehub} from "../../../bridgehub/IBridgehub.sol";
 import {UncheckedMath} from "../../../common/libraries/UncheckedMath.sol";
 import {IGetters} from "../../chain-interfaces/IGetters.sol";
 import {ILegacyGetters} from "../../chain-interfaces/ILegacyGetters.sol";
-import {InvalidSelector} from "../../../common/L1ContractErrors.sol";
 import {SemVer} from "../../../common/libraries/SemVer.sol";
 
 // While formally the following import is not used, it is needed to inherit documentation from it