From b747e516bbdaf8745be92d2f4b11fb9e63cf7a05 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C3=A9ter=20Garamv=C3=B6lgyi?= Date: Wed, 11 Feb 2026 12:49:49 +0100 Subject: [PATCH 1/2] feat(validium): disallow arbitrary from address in ERC20 gateway --- foundry.lock | 11 ++++ src/test/validium/FastWithdrawVault.t.sol | 3 +- .../validium/L1ERC20GatewayValidium.t.sol | 52 +++++++++++++------ src/test/validium/L1WETHGatewayValidium.t.sol | 34 ++++++------ src/validium/L1ERC20GatewayValidium.sol | 15 +++++- 5 files changed, 78 insertions(+), 37 deletions(-) create mode 100644 foundry.lock diff --git a/foundry.lock b/foundry.lock new file mode 100644 index 00000000..e12d3a68 --- /dev/null +++ b/foundry.lock @@ -0,0 +1,11 @@ +{ + "lib/ds-test": { + "rev": "e282159d5170298eb2455a6c05280ab5a73a4ef0" + }, + "lib/forge-std": { + "rev": "978ac6fadb62f5f0b723c996f64be52eddba6801" + }, + "lib/solmate": { + "rev": "c892309933b25c03d32b1b0d674df7ae292ba925" + } +} \ No newline at end of file diff --git a/src/test/validium/FastWithdrawVault.t.sol b/src/test/validium/FastWithdrawVault.t.sol index 92930e3e..860f4b60 100644 --- a/src/test/validium/FastWithdrawVault.t.sol +++ b/src/test/validium/FastWithdrawVault.t.sol @@ -268,7 +268,8 @@ contract FastWithdrawVaultTest is ValidiumTestBase { address(messenger), address(template), address(factory), - address(rollup) + address(rollup), + address(1) // wethGateway - placeholder for tests ) ) ); diff --git a/src/test/validium/L1ERC20GatewayValidium.t.sol b/src/test/validium/L1ERC20GatewayValidium.t.sol index fd6efaed..a1a537df 100644 --- a/src/test/validium/L1ERC20GatewayValidium.t.sol +++ b/src/test/validium/L1ERC20GatewayValidium.t.sol @@ -14,6 +14,7 @@ import {AddressAliasHelper} from "../../libraries/common/AddressAliasHelper.sol" import {IL1ERC20GatewayValidium} from "../../validium/IL1ERC20GatewayValidium.sol"; import {IL2ERC20GatewayValidium} from "../../validium/IL2ERC20GatewayValidium.sol"; import {L1ERC20GatewayValidium} from "../../validium/L1ERC20GatewayValidium.sol"; +import {L1WETHGatewayValidium} from "../../validium/L1WethGatewayValidium.sol"; import {ScrollChainValidium} from "../../validium/ScrollChainValidium.sol"; import {TransferReentrantToken} from "../mocks/tokens/TransferReentrantToken.sol"; @@ -46,6 +47,7 @@ contract L1ERC20GatewayValidiumTest is ValidiumTestBase { ScrollStandardERC20 private template; ScrollStandardERC20Factory private factory; L2StandardERC20Gateway private counterpartGateway; + L1WETHGatewayValidium private wethGateway = L1WETHGatewayValidium(address(1)); // placeholder for tests MockERC20 private l1Token; MockERC20 private l2Token; @@ -120,23 +122,43 @@ contract L1ERC20GatewayValidiumTest is ValidiumTestBase { _deposit(address(this), amount, recipient, gasLimit); } - function testDepositERC20WithSender( - address sender, + function testDepositERC20WrongKey( uint256 amount, bytes memory recipient, uint256 gasLimit ) public { - _deposit(sender, amount, recipient, gasLimit); + (uint256 keyId, ) = rollup.getLatestEncryptionKey(); + hevm.expectRevert(ScrollChainValidium.ErrorUnknownEncryptionKey.selector); + gateway.depositERC20(address(l1Token), recipient, amount, gasLimit, keyId + 1); } - function testDepositERC20WrongKey( + function testDepositERC20WithRealSenderUnauthorized( + address attacker, + address victim, uint256 amount, bytes memory recipient, uint256 gasLimit ) public { + hevm.assume(attacker != address(0)); + hevm.assume(attacker != address(wethGateway)); + + amount = bound(amount, 1, l1Token.balanceOf(address(this))); + gasLimit = bound(gasLimit, defaultGasLimit / 2, defaultGasLimit); (uint256 keyId, ) = rollup.getLatestEncryptionKey(); - hevm.expectRevert(ScrollChainValidium.ErrorUnknownEncryptionKey.selector); - gateway.depositERC20(address(l1Token), recipient, amount, gasLimit, keyId + 1); + + // Transfer tokens to attacker + l1Token.transfer(attacker, amount); + + // Attacker approves gateway + hevm.startPrank(attacker); + l1Token.approve(address(gateway), amount); + + // Attacker tries to call depositERC20 with victim as _realSender + // This should revert with ErrorCallerNotWethGateway + hevm.expectRevert(L1ERC20GatewayValidium.ErrorCallerNotWethGateway.selector); + gateway.depositERC20(address(l1Token), victim, recipient, amount, gasLimit, keyId); + + hevm.stopPrank(); } function testDepositReentrantToken(uint256 amount) public { @@ -407,12 +429,11 @@ contract L1ERC20GatewayValidiumTest is ValidiumTestBase { if (amount == 0) { (uint256 keyId, ) = rollup.getLatestEncryptionKey(); hevm.expectRevert(L1ERC20GatewayValidium.ErrorAmountIsZero.selector); - if (from == address(this)) { - gateway.depositERC20(address(l1Token), recipient, amount, gasLimit, keyId); - } else { - gateway.depositERC20(address(l1Token), from, recipient, amount, gasLimit, keyId); - } + gateway.depositERC20(address(l1Token), recipient, amount, gasLimit, keyId); } else { + // Note: from parameter is only used in event expectations, not in actual calls + // The depositERC20 function always uses msg.sender for actual deposits + // emit QueueTransaction from L1MessageQueueV2 { hevm.expectEmit(true, true, false, true); @@ -434,11 +455,7 @@ contract L1ERC20GatewayValidiumTest is ValidiumTestBase { uint256 feeVaultBalance = address(feeVault).balance; assertEq(l1Messenger.messageSendTimestamp(keccak256(xDomainCalldata)), 0); (uint256 keyId, ) = rollup.getLatestEncryptionKey(); - if (from == address(this)) { - gateway.depositERC20(address(l1Token), recipient, amount, gasLimit, keyId); - } else { - gateway.depositERC20(address(l1Token), from, recipient, amount, gasLimit, keyId); - } + gateway.depositERC20(address(l1Token), recipient, amount, gasLimit, keyId); assertEq(amount + gatewayBalance, l1Token.balanceOf(address(gateway))); assertEq(feeVaultBalance, address(feeVault).balance); assertGt(l1Messenger.messageSendTimestamp(keccak256(xDomainCalldata)), 0); @@ -456,7 +473,8 @@ contract L1ERC20GatewayValidiumTest is ValidiumTestBase { address(messenger), address(template), address(factory), - address(rollup) + address(rollup), + address(wethGateway) ) ) ); diff --git a/src/test/validium/L1WETHGatewayValidium.t.sol b/src/test/validium/L1WETHGatewayValidium.t.sol index 53b9ff5b..d5589ec2 100644 --- a/src/test/validium/L1WETHGatewayValidium.t.sol +++ b/src/test/validium/L1WETHGatewayValidium.t.sol @@ -64,9 +64,24 @@ contract L1WETHGatewayValidiumTest is ValidiumTestBase { counterpartGateway = new L2StandardERC20Gateway(address(1), address(1), address(1), address(factory)); // Deploy L1 contracts - gateway = _deployGateway(address(l1Messenger)); + gateway = L1ERC20GatewayValidium(_deployProxy(address(0))); wethGateway = new L1WETHGatewayValidium(address(weth), address(gateway)); + // Upgrade gateway implementation with actual wethGateway address + admin.upgrade( + ITransparentUpgradeableProxy(address(gateway)), + address( + new L1ERC20GatewayValidium( + address(counterpartGateway), + address(l1Messenger), + address(template), + address(factory), + address(rollup), + address(wethGateway) + ) + ) + ); + // Initialize L1 contracts gateway.initialize(); @@ -146,21 +161,4 @@ contract L1WETHGatewayValidiumTest is ValidiumTestBase { assertGt(l1Messenger.messageSendTimestamp(keccak256(xDomainCalldata)), 0); } } - - function _deployGateway(address messenger) internal returns (L1ERC20GatewayValidium _gateway) { - _gateway = L1ERC20GatewayValidium(_deployProxy(address(0))); - - admin.upgrade( - ITransparentUpgradeableProxy(address(_gateway)), - address( - new L1ERC20GatewayValidium( - address(counterpartGateway), - address(messenger), - address(template), - address(factory), - address(rollup) - ) - ) - ); - } } diff --git a/src/validium/L1ERC20GatewayValidium.sol b/src/validium/L1ERC20GatewayValidium.sol index 1580e439..fa8e1112 100644 --- a/src/validium/L1ERC20GatewayValidium.sol +++ b/src/validium/L1ERC20GatewayValidium.sol @@ -34,6 +34,9 @@ contract L1ERC20GatewayValidium is ScrollGatewayBase, IL1ERC20GatewayValidium { /// @dev Error thrown when amount is zero. error ErrorAmountIsZero(); + /// @dev Error thrown when caller is not the WETH gateway. + error ErrorCallerNotWethGateway(); + /************* * Constants * *************/ @@ -47,6 +50,9 @@ contract L1ERC20GatewayValidium is ScrollGatewayBase, IL1ERC20GatewayValidium { /// @notice The address of ScrollChainValidium contract in L2. address public immutable scrollChainValidium; + /// @notice The address of L1WETHGatewayValidium contract. + address public immutable wethGateway; + /************* * Variables * *************/ @@ -67,18 +73,22 @@ contract L1ERC20GatewayValidium is ScrollGatewayBase, IL1ERC20GatewayValidium { /// @param _messenger The address of `L1ScrollMessenger` contract in L1. /// @param _l2TokenImplementation The address of `ScrollStandardERC20` implementation in L2. /// @param _l2TokenFactory The address of `ScrollStandardERC20Factory` contract in L2. + /// @param _scrollChainValidium The address of `ScrollChainValidium` contract in L2. + /// @param _wethGateway The address of `L1WETHGatewayValidium` contract. constructor( address _counterpart, address _messenger, address _l2TokenImplementation, address _l2TokenFactory, - address _scrollChainValidium + address _scrollChainValidium, + address _wethGateway ) ScrollGatewayBase(_counterpart, address(0), _messenger) { _disableInitializers(); l2TokenImplementation = _l2TokenImplementation; l2TokenFactory = _l2TokenFactory; scrollChainValidium = _scrollChainValidium; + wethGateway = _wethGateway; } /// @notice Initialize the storage of L1ERC20GatewayValidium. @@ -123,6 +133,9 @@ contract L1ERC20GatewayValidium is ScrollGatewayBase, IL1ERC20GatewayValidium { uint256 _gasLimit, uint256 _keyId ) external payable override { + // Only the WETH gateway can call this function to preserve the real sender + if (_msgSender() != wethGateway) revert ErrorCallerNotWethGateway(); + _deposit(_token, _realSender, _to, _amount, new bytes(0), _gasLimit, _keyId); } From 826601dd0a36c9127ea42886cf0ea27328414d22 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C3=A9ter=20Garamv=C3=B6lgyi?= Date: Wed, 11 Feb 2026 12:53:15 +0100 Subject: [PATCH 2/2] fix path --- src/test/validium/L1ERC20GatewayValidium.t.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/validium/L1ERC20GatewayValidium.t.sol b/src/test/validium/L1ERC20GatewayValidium.t.sol index a1a537df..5961ad17 100644 --- a/src/test/validium/L1ERC20GatewayValidium.t.sol +++ b/src/test/validium/L1ERC20GatewayValidium.t.sol @@ -14,7 +14,7 @@ import {AddressAliasHelper} from "../../libraries/common/AddressAliasHelper.sol" import {IL1ERC20GatewayValidium} from "../../validium/IL1ERC20GatewayValidium.sol"; import {IL2ERC20GatewayValidium} from "../../validium/IL2ERC20GatewayValidium.sol"; import {L1ERC20GatewayValidium} from "../../validium/L1ERC20GatewayValidium.sol"; -import {L1WETHGatewayValidium} from "../../validium/L1WethGatewayValidium.sol"; +import {L1WETHGatewayValidium} from "../../validium/L1WETHGatewayValidium.sol"; import {ScrollChainValidium} from "../../validium/ScrollChainValidium.sol"; import {TransferReentrantToken} from "../mocks/tokens/TransferReentrantToken.sol";