spearbit review fix

alchemyplatform · Feb 13, 2024 · 9e6002a · 9e6002a
1 parent fd4b721
commit 9e6002a
Show file tree

Hide file tree

Showing 2 changed files with 4 additions and 5 deletions.
diff --git a/src/plugins/session/SessionKeyPlugin.sol b/src/plugins/session/SessionKeyPlugin.sol
@@ -193,9 +193,10 @@ contract SessionKeyPlugin is ISessionKeyPlugin, SessionKeyPermissions, BasePlugi
             }
 
             uint256 validation = _checkUserOpPermissions(userOp, calls, sessionKey);
-            if (uint160(validation) > 0) {
+            if (uint160(validation) != uint160(SIG_VALIDATION_PASSED)) {
                 revert PermissionsCheckFailed();
             }
+            // return SIG_VALIDATION_FAILED on sig validation failure only, all other failure modes should revert
             return validation | (sessionKey == recoveredSig ? SIG_VALIDATION_PASSED : SIG_VALIDATION_FAILED);
         }
         revert NotImplemented(msg.sig, functionId);

diff --git a/src/plugins/session/permissions/SessionKeyPermissions.sol b/src/plugins/session/permissions/SessionKeyPermissions.sol
@@ -140,10 +140,8 @@ abstract contract SessionKeyPermissions is ISessionKeyPlugin, SessionKeyPermissi
             address userOpPaymaster = address(bytes20(userOp.paymasterAndData));
             validationSuccess = validationSuccess && (userOpPaymaster == sessionKeyData.requiredPaymaster);
         }
-        // Validation return data is 1 in the case of an invalid signature,
-        // otherwise a packed struct of the aggregator address (0 here), and two
-        // 6-byte timestamps indicating the start and end times at which the op
-        // is valid.
+        // A packed struct of the SIG_VALIDATION_PASSED or SIG_VALIDATION_FAILED, and two
+        // 6-byte timestamps indicating the start and end times at which the op is valid.
         return uint160(validationSuccess ? SIG_VALIDATION_PASSED : SIG_VALIDATION_FAILED)
             | (uint256(validUntil) << 160) | (uint256(currentValidAfter) << (208));
     }