From 5b97c01e61568f1dabd7d53fa3f209bdb1db661a Mon Sep 17 00:00:00 2001 From: nicholaspai Date: Wed, 15 Mar 2023 12:36:52 -0400 Subject: [PATCH] fix: [L04] updatedRelayerFeePct can be lower than expected ## From Audit In the SpokePool contract, the speedUpDeposit function performs the following check on the updatedRelayerFeePct variable: require(updatedRelayerFeePct < 0.5e18, "invalid relayer fee"); Unlike the equivalent check in the deposit function, in this case the SignedMath.abs function is not used, which allows the updatedRelayerFeePct value to reach or go below the lower limit of -0.5e18 . An attempt by the relayer to fill this request will be rejected by the _fillRelay function's fee check. Consider using the SignedMath.abs function to ensure the updatedRelayerFeePct argument provided to speedUpDeposit is within the expected limits. --- .eslintignore | 2 ++ .npmignore | 1 + .prettierignore | 2 ++ contracts/SpokePool.sol | 2 +- test/SpokePool.Relay.ts | 16 ++++++++++++++-- 5 files changed, 20 insertions(+), 3 deletions(-) diff --git a/.eslintignore b/.eslintignore index dfd3a0917..8a2103827 100644 --- a/.eslintignore +++ b/.eslintignore @@ -4,3 +4,5 @@ cache coverage* gasReporterOutput.json typechain +artifacts-zk +cache-zk diff --git a/.npmignore b/.npmignore index c4ff6d397..f6ef6a5b9 100644 --- a/.npmignore +++ b/.npmignore @@ -1,2 +1,3 @@ hardhat.config.ts cache +cache-zk \ No newline at end of file diff --git a/.prettierignore b/.prettierignore index 77d708c34..59766e79e 100644 --- a/.prettierignore +++ b/.prettierignore @@ -5,3 +5,5 @@ coverage* gasReporterOutput.json typechain dist +artifacts-zk +cache-zk diff --git a/contracts/SpokePool.sol b/contracts/SpokePool.sol index 79488dee6..01660c33b 100644 --- a/contracts/SpokePool.sol +++ b/contracts/SpokePool.sol @@ -460,7 +460,7 @@ abstract contract SpokePool is bytes memory updatedMessage, bytes memory depositorSignature ) public override nonReentrant { - require(updatedRelayerFeePct < 0.5e18, "invalid relayer fee"); + require(SignedMath.abs(updatedRelayerFeePct) < 0.5e18, "Invalid relayer fee"); _verifyUpdateDepositMessage( depositor, diff --git a/test/SpokePool.Relay.ts b/test/SpokePool.Relay.ts index 2f376555b..920ea35ef 100644 --- a/test/SpokePool.Relay.ts +++ b/test/SpokePool.Relay.ts @@ -303,7 +303,7 @@ async function testUpdatedFeeSignaling(depositorAddress: string) { updatedMessage ); - // Cannot set new relayer fee pct >= 50% + // Cannot set new relayer fee pct >= 50% or <= -50% await expect( spokePool .connect(relayer) @@ -315,7 +315,19 @@ async function testUpdatedFeeSignaling(depositorAddress: string) { updatedMessage, signature ) - ).to.be.revertedWith("invalid relayer fee"); + ).to.be.revertedWith("Invalid relayer fee"); + await expect( + spokePool + .connect(relayer) + .speedUpDeposit( + depositorAddress, + toWei("0.5").mul(-1), + consts.firstDepositId, + updatedRecipient, + updatedMessage, + signature + ) + ).to.be.revertedWith("Invalid relayer fee"); await expect( spokePool