fix: [QS-12] prevent setting bad time ranges (#286)

alchemyplatform · Nov 14, 2024 · 4e7e073 · 4e7e073
1 parent 357cb8c
commit 4e7e073
Show file tree

Hide file tree

Showing 4 changed files with 30 additions and 10 deletions.
diff --git a/gas-snapshots/ModularAccount.json b/gas-snapshots/ModularAccount.json
@@ -2,13 +2,13 @@
   "Runtime_AccountCreation": "176125",
   "Runtime_BatchTransfers": "92985",
   "Runtime_Erc20Transfer": "78394",
-  "Runtime_InstallSessionKey_Case1": "423008",
+  "Runtime_InstallSessionKey_Case1": "423036",
   "Runtime_NativeTransfer": "54543",
   "Runtime_UseSessionKey_Case1_Counter": "78606",
   "Runtime_UseSessionKey_Case1_Token": "111919",
   "UserOp_BatchTransfers": "198306",
   "UserOp_Erc20Transfer": "185271",
-  "UserOp_InstallSessionKey_Case1": "531215",
+  "UserOp_InstallSessionKey_Case1": "531243",
   "UserOp_NativeTransfer": "161528",
   "UserOp_UseSessionKey_Case1_Counter": "195233",
   "UserOp_UseSessionKey_Case1_Token": "226217",

diff --git a/gas-snapshots/SemiModularAccount.json b/gas-snapshots/SemiModularAccount.json
@@ -2,13 +2,13 @@
   "Runtime_AccountCreation": "97745",
   "Runtime_BatchTransfers": "88968",
   "Runtime_Erc20Transfer": "74425",
-  "Runtime_InstallSessionKey_Case1": "421557",
+  "Runtime_InstallSessionKey_Case1": "421585",
   "Runtime_NativeTransfer": "50584",
   "Runtime_UseSessionKey_Case1_Counter": "78956",
   "Runtime_UseSessionKey_Case1_Token": "112269",
   "UserOp_BatchTransfers": "193459",
   "UserOp_Erc20Transfer": "180495",
-  "UserOp_InstallSessionKey_Case1": "528735",
+  "UserOp_InstallSessionKey_Case1": "528763",
   "UserOp_NativeTransfer": "156770",
   "UserOp_UseSessionKey_Case1_Counter": "195473",
   "UserOp_UseSessionKey_Case1_Token": "226457",

diff --git a/src/modules/permissions/TimeRangeModule.sol b/src/modules/permissions/TimeRangeModule.sol
@@ -112,6 +112,9 @@ contract TimeRangeModule is IValidationHookModule, ModuleBase {
     /// @param validUntil The timestamp until which the time range is valid, inclusive.
     /// @param validAfter The timestamp after which the time range is valid, inclusive.
     function setTimeRange(uint32 entityId, uint48 validUntil, uint48 validAfter) public {
+        if (validUntil <= validAfter) {
+            revert TimeRangeNotValid();
+        }
         timeRanges[entityId][msg.sender] = TimeRange(validUntil, validAfter);
     }
 

diff --git a/test/modules/TimeRangeModule.t.sol b/test/modules/TimeRangeModule.t.sol
@@ -105,9 +105,25 @@ contract TimeRangeModuleTest is CustomValidationTestBase {
         assertEq(retrievedValidAfter, 0);
     }
 
-    function testFuzz_timeRangeModule_userOp(uint48 _validUntil, uint48 _validAfter) public {
-        validUntil = _validUntil;
-        validAfter = _validAfter;
+    function test_timeRangeModule_setBadTime() public withSMATest {
+        validUntil = 1000;
+        validAfter = 100;
+
+        _customValidationSetup();
+
+        vm.startPrank(address(account1));
+        vm.expectRevert(TimeRangeModule.TimeRangeNotValid.selector);
+        timeRangeModule.setTimeRange(TEST_DEFAULT_VALIDATION_ENTITY_ID, validUntil, validUntil);
+
+        vm.expectRevert(TimeRangeModule.TimeRangeNotValid.selector);
+        timeRangeModule.setTimeRange(TEST_DEFAULT_VALIDATION_ENTITY_ID, validUntil, validUntil + 1);
+        vm.stopPrank();
+    }
+
+    function testFuzz_timeRangeModule_userOp(uint48 time1, uint48 time2) public {
+        vm.assume(time1 != time2);
+        validUntil = time1 > time2 ? time1 : time2;
+        validAfter = time1 < time2 ? time1 : time2;
 
         _customValidationSetup();
 
@@ -140,9 +156,10 @@ contract TimeRangeModuleTest is CustomValidationTestBase {
         );
     }
 
-    function testFuzz_timeRangeModule_userOp_fail(uint48 _validUntil, uint48 _validAfter) public {
-        validUntil = _validUntil;
-        validAfter = _validAfter;
+    function testFuzz_timeRangeModule_userOp_fail(uint48 time1, uint48 time2) public {
+        vm.assume(time1 != time2);
+        validUntil = time1 > time2 ? time1 : time2;
+        validAfter = time1 < time2 ? time1 : time2;
 
         _customValidationSetup();