From 9e6002a263842aa0a66e8d8058ddb42aa154ed58 Mon Sep 17 00:00:00 2001
From: howydev <132113803+howydev@users.noreply.github.com>
Date: Tue, 13 Feb 2024 14:10:59 -0500
Subject: [PATCH] spearbit review fix

---
 src/plugins/session/SessionKeyPlugin.sol                  | 3 ++-
 src/plugins/session/permissions/SessionKeyPermissions.sol | 6 ++----
 2 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/src/plugins/session/SessionKeyPlugin.sol b/src/plugins/session/SessionKeyPlugin.sol
index 1ba0d7fa..a5fe0540 100644
--- 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
index f8a42ed0..3d47af48 100644
--- 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));
     }