From 45bb1326158323b768f186d9a6ade242598df3ef Mon Sep 17 00:00:00 2001 From: Melanciani Date: Tue, 30 Dec 2025 12:14:49 +0100 Subject: [PATCH] fix(protocol-contracts): wrap permit call with try-catch to prevent frontrun DOS (L-04) --- .../staking/contracts/OperatorStaking.sol | 3 ++- .../staking/test/OperatorStaking.test.ts | 14 ++++++++++---- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/protocol-contracts/staking/contracts/OperatorStaking.sol b/protocol-contracts/staking/contracts/OperatorStaking.sol index 2f047261d3..1c5f61626a 100644 --- a/protocol-contracts/staking/contracts/OperatorStaking.sol +++ b/protocol-contracts/staking/contracts/OperatorStaking.sol @@ -160,7 +160,8 @@ contract OperatorStaking is ERC1363Upgradeable, ReentrancyGuardTransient, UUPSUp bytes32 r, bytes32 s ) public virtual returns (uint256) { - IERC20Permit(asset()).permit(msg.sender, address(this), assets, deadline, v, r, s); + // Use try-catch to prevent frontrun DOS attacks on permit (see ERC-2612) + try IERC20Permit(asset()).permit(msg.sender, address(this), assets, deadline, v, r, s) {} catch {} return deposit(assets, receiver); } diff --git a/protocol-contracts/staking/test/OperatorStaking.test.ts b/protocol-contracts/staking/test/OperatorStaking.test.ts index 086f48f0d7..630ee575df 100644 --- a/protocol-contracts/staking/test/OperatorStaking.test.ts +++ b/protocol-contracts/staking/test/OperatorStaking.test.ts @@ -190,20 +190,26 @@ describe('OperatorStaking', function () { .withArgs(this.delegatorNoApproval, this.mock, this.permitValue); }); - it('should revert if signature is invalid', async function () { + it('should revert with insufficient allowance if signature is invalid', async function () { + // With try-catch on permit, invalid signature causes permit to silently fail, + // then deposit fails with ERC20InsufficientAllowance since no approval exists await expect( this.mock .connect(this.delegatorNoApproval) .depositWithPermit(this.permitValue, this.delegatorNoApproval, this.permitDeadline, 0, this.r, this.s), - ).to.be.revertedWithCustomError(this.token, 'ECDSAInvalidSignature'); + ).to.be.revertedWithCustomError(this.token, 'ERC20InsufficientAllowance'); }); - it('should revert if signer is invalid', async function () { + it('should succeed if signer has existing approval even with invalid permit', async function () { + // With try-catch on permit, invalid signer causes permit to silently fail, + // but delegator1 already has approval so deposit succeeds await expect( this.mock .connect(this.delegator1) .depositWithPermit(this.permitValue, this.delegator1, this.permitDeadline, this.v, this.r, this.s), - ).to.be.revertedWithCustomError(this.token, 'ERC2612InvalidSigner'); + ) + .to.emit(this.mock, 'Transfer') + .withArgs(ethers.ZeroAddress, this.delegator1, this.permitValue); }); });