refactor: new signature frontrun fix

morpho-org · Aug 11, 2023 · 5e84d6c · 5e84d6c
1 parent 75245f4
commit 5e84d6c
Show file tree

Hide file tree

Showing 4 changed files with 23 additions and 40 deletions.
diff --git a/src/Blue.sol b/src/Blue.sol
@@ -346,30 +346,21 @@ contract Blue is IBlue {
     }
 
     /// @dev The signature is malleable, but it has no impact on the security here.
-    function setAuthorizationWithSig(
-        address authorizer,
-        address authorized,
-        bool newIsAuthorized,
-        uint256 deadline,
-        Signature calldata signature
-    ) external {
-        require(block.timestamp < deadline, ErrorsLib.SIGNATURE_EXPIRED);
-
-        uint256 usedNonce = nonce[authorizer];
-        bytes32 hashStruct =
-            keccak256(abi.encode(AUTHORIZATION_TYPEHASH, authorizer, authorized, newIsAuthorized, usedNonce, deadline));
+    function setAuthorizationWithSig(Authorization calldata authorization, Signature calldata signature) external {
+        require(block.timestamp < authorization.deadline, ErrorsLib.SIGNATURE_EXPIRED);
+
+        bytes32 hashStruct = keccak256(abi.encode(AUTHORIZATION_TYPEHASH, authorization));
         bytes32 digest = keccak256(abi.encodePacked("\x19\x01", DOMAIN_SEPARATOR, hashStruct));
         address signatory = ecrecover(digest, signature.v, signature.r, signature.s);
 
-        if (signatory != address(0) && authorizer == signatory) {
-            nonce[authorizer]++;
-            emit EventsLib.IncrementNonce(msg.sender, authorizer, usedNonce);
+        require(signatory != address(0) && authorization.authorizer == signatory, ErrorsLib.INVALID_SIGNATURE);
 
-            isAuthorized[authorizer][authorized] = newIsAuthorized;
-            emit EventsLib.SetAuthorization(msg.sender, authorizer, authorized, newIsAuthorized);
-        } else {
-            // To avoid potential frontrun attacks, when the signature is not verified, only prevent attempts that actually change the authorization.
-            require(isAuthorized[authorizer][authorized] == newIsAuthorized, ErrorsLib.INVALID_SIGNATURE);
+        if (authorization.nonce == nonce[authorization.authorizer]) {
+            emit EventsLib.IncrementNonce(msg.sender, authorization.authorizer, nonce[authorization.authorizer]++);
+            isAuthorized[authorization.authorizer][authorization.authorized] = authorization.isAuthorized;
+            emit EventsLib.SetAuthorization(
+                msg.sender, authorization.authorizer, authorization.authorized, authorization.isAuthorized
+            );
         }
     }
 

diff --git a/src/interfaces/IBlue.sol b/src/interfaces/IBlue.sol
@@ -13,6 +13,14 @@ struct Market {
     uint256 lltv;
 }
 
+struct Authorization {
+    address authorizer;
+    address authorized;
+    bool isAuthorized;
+    uint256 nonce;
+    uint256 deadline;
+}
+
 /// @notice Contains the `v`, `r` and `s` parameters of an ECDSA signature.
 struct Signature {
     uint8 v;
@@ -62,13 +70,7 @@ interface IBlue is IFlashLender {
     function flashLoan(address token, uint256 amount, bytes calldata data) external;
 
     function setAuthorization(address authorized, bool newIsAuthorized) external;
-    function setAuthorizationWithSig(
-        address authorizer,
-        address authorized,
-        bool newIsAuthorized,
-        uint256 deadline,
-        Signature calldata signature
-    ) external;
+    function setAuthorizationWithSig(Authorization calldata authorization, Signature calldata signature) external;
 
     function extsload(bytes32[] memory slots) external view returns (bytes32[] memory res);
 }
diff --git a/test/forge/Blue.t.sol b/test/forge/Blue.t.sol
@@ -858,7 +858,7 @@ contract BlueTest is
         privateKey = bound(privateKey, 1, type(uint32).max); // "Private key must be less than the secp256k1 curve order (115792089237316195423570985008687907852837564279074904382605163141518161494337)."
         address authorizer = vm.addr(privateKey);
 
-        SigUtils.Authorization memory authorization = SigUtils.Authorization({
+        Authorization memory authorization = Authorization({
             authorizer: authorizer,
             authorized: authorized,
             isAuthorized: isAuthorized,
@@ -871,9 +871,7 @@ contract BlueTest is
         Signature memory sig;
         (sig.v, sig.r, sig.s) = vm.sign(privateKey, digest);
 
-        blue.setAuthorizationWithSig(
-            authorization.authorizer, authorization.authorized, authorization.isAuthorized, authorization.deadline, sig
-        );
+        blue.setAuthorizationWithSig(authorization, sig);
 
         assertEq(blue.isAuthorized(authorizer, authorized), isAuthorized);
         assertEq(blue.nonce(authorizer), 1);

diff --git a/test/forge/helpers/SigUtils.sol b/test/forge/helpers/SigUtils.sol
@@ -1,17 +1,9 @@
 // SPDX-License-Identifier: UNLICENSED
 pragma solidity ^0.8.0;
 
-import {AUTHORIZATION_TYPEHASH} from "src/Blue.sol";
+import {Authorization, AUTHORIZATION_TYPEHASH} from "src/Blue.sol";
 
 library SigUtils {
-    struct Authorization {
-        address authorizer;
-        address authorized;
-        bool isAuthorized;
-        uint256 nonce;
-        uint256 deadline;
-    }
-
     /// @dev Computes the hash of the EIP-712 encoded data.
     function getTypedDataHash(bytes32 domainSeparator, Authorization memory authorization)
         public