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); }); });