From 4c7e25f02940cbfe70788c835b4e7c05fc048768 Mon Sep 17 00:00:00 2001 From: MathisGD Date: Sun, 13 Aug 2023 12:10:49 +0200 Subject: [PATCH 1/2] refactor: setAuthWithSig + add warning message about revert --- src/Morpho.sol | 27 ++++++++------------ src/interfaces/IMorpho.sol | 31 +++++++++++++---------- test/forge/Morpho.t.sol | 45 ++++++++++++++++++--------------- test/forge/helpers/SigUtils.sol | 10 +------- 4 files changed, 55 insertions(+), 58 deletions(-) diff --git a/src/Morpho.sol b/src/Morpho.sol index 84d9d212b..e081e73e9 100644 --- a/src/Morpho.sol +++ b/src/Morpho.sol @@ -381,29 +381,24 @@ contract Morpho is IMorpho { } /// @inheritdoc IMorpho + /// @dev Warning: reverts if the signature has already been submitted. /// @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 memory 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); - require(signatory != address(0) && authorizer == signatory, ErrorsLib.INVALID_SIGNATURE); + require(signatory != address(0) && authorization.authorizer == signatory, ErrorsLib.INVALID_SIGNATURE); - emit EventsLib.IncrementNonce(msg.sender, authorizer, usedNonce); + emit EventsLib.IncrementNonce(msg.sender, authorization.authorizer, nonce[authorization.authorizer]++); - isAuthorized[authorizer][authorized] = newIsAuthorized; + isAuthorized[authorization.authorizer][authorization.authorized] = authorization.isAuthorized; - emit EventsLib.SetAuthorization(msg.sender, authorizer, authorized, newIsAuthorized); + emit EventsLib.SetAuthorization( + msg.sender, authorization.authorizer, authorization.authorized, authorization.isAuthorized + ); } function _isSenderAuthorized(address user) internal view returns (bool) { diff --git a/src/interfaces/IMorpho.sol b/src/interfaces/IMorpho.sol index a2628732e..119db889a 100644 --- a/src/interfaces/IMorpho.sol +++ b/src/interfaces/IMorpho.sol @@ -19,6 +19,20 @@ struct Market { uint256 lltv; } +/// @notice Authorization struct. +/// @param authorizer Authorizer address. +/// @param authorized Authorized address. +/// @param isAuthorized isAuthorized to set. +/// @param nonce Signature nonce. +/// @param deadline Signature deadline. +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; @@ -198,19 +212,10 @@ interface IMorpho is IFlashLender { /// @param newIsAuthorized The new authorization status. function setAuthorization(address authorized, bool newIsAuthorized) external; - /// @notice Sets the authorization for `authorized` to manage `authorizer`'s positions. - /// @param authorizer The authorizer address. - /// @param authorized The authorized address. - /// @param newIsAuthorized The new authorization status. - /// @param deadline The deadline after which the signature is invalid. - /// @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; + /// @notice Sets the authorization for `authorization.authorized` to manage `authorization.authorizer`'s positions. + /// @param authorization The Authorization struct. + /// @param signature The signature. + function setAuthorizationWithSig(Authorization calldata authorization, Signature calldata signature) external; /// @notice Returns the data stored on the different `slots`. function extsload(bytes32[] memory slots) external view returns (bytes32[] memory res); diff --git a/test/forge/Morpho.t.sol b/test/forge/Morpho.t.sol index a3ca46f4c..052f5705f 100644 --- a/test/forge/Morpho.t.sol +++ b/test/forge/Morpho.t.sol @@ -853,32 +853,37 @@ contract MorphoTest is vm.stopPrank(); } - function testAuthorizationWithSig(uint32 deadline, address authorized, uint256 privateKey, bool isAuthorized) - public - { - deadline = uint32(bound(deadline, block.timestamp + 1, type(uint32).max)); - 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({ - authorizer: authorizer, - authorized: authorized, - isAuthorized: isAuthorized, - nonce: morpho.nonce(authorizer), - deadline: block.timestamp + deadline - }); + function testAuthorizationWithSig(Authorization memory authorization, uint256 privateKey) public { + vm.assume(authorization.deadline > block.timestamp); - bytes32 digest = SigUtils.getTypedDataHash(morpho.DOMAIN_SEPARATOR(), authorization); + // Private key must be less than the secp256k1 curve order. + privateKey = bound(privateKey, 1, type(uint32).max); + authorization.nonce = 0; + authorization.authorizer = vm.addr(privateKey); Signature memory sig; + bytes32 digest = SigUtils.getTypedDataHash(morpho.DOMAIN_SEPARATOR(), authorization); (sig.v, sig.r, sig.s) = vm.sign(privateKey, digest); - morpho.setAuthorizationWithSig( - authorization.authorizer, authorization.authorized, authorization.isAuthorized, authorization.deadline, sig - ); + morpho.setAuthorizationWithSig(authorization, sig); + + assertEq(morpho.isAuthorized(authorization.authorizer, authorization.authorized), authorization.isAuthorized); + assertEq(morpho.nonce(authorization.authorizer), 1); + } + + function testAuthorizationWithSigWrongPK(Authorization memory authorization, uint256 privateKey) public { + vm.assume(authorization.deadline > block.timestamp); + + // Private key must be less than the secp256k1 curve order. + privateKey = bound(privateKey, 1, type(uint32).max); + authorization.nonce = 0; + + Signature memory sig; + bytes32 digest = SigUtils.getTypedDataHash(morpho.DOMAIN_SEPARATOR(), authorization); + (sig.v, sig.r, sig.s) = vm.sign(privateKey, digest); - assertEq(morpho.isAuthorized(authorizer, authorized), isAuthorized); - assertEq(morpho.nonce(authorizer), 1); + vm.expectRevert(bytes(ErrorsLib.INVALID_SIGNATURE)); + morpho.setAuthorizationWithSig(authorization, sig); } function testFlashLoan(uint256 assets) public { diff --git a/test/forge/helpers/SigUtils.sol b/test/forge/helpers/SigUtils.sol index f85baa46c..b7972534f 100644 --- 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/Morpho.sol"; +import {Authorization, AUTHORIZATION_TYPEHASH} from "src/Morpho.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 From 9030a7191a1f25fddb255195fa9518d750e07f15 Mon Sep 17 00:00:00 2001 From: MathisGD <74971347+MathisGD@users.noreply.github.com> Date: Sun, 13 Aug 2023 18:14:14 +0200 Subject: [PATCH 2/2] docs: improve authorization doc Co-authored-by: Merlin Egalite <44097430+MerlinEgalite@users.noreply.github.com> Signed-off-by: MathisGD <74971347+MathisGD@users.noreply.github.com> --- src/interfaces/IMorpho.sol | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/interfaces/IMorpho.sol b/src/interfaces/IMorpho.sol index 119db889a..101f7aa8b 100644 --- a/src/interfaces/IMorpho.sol +++ b/src/interfaces/IMorpho.sol @@ -22,7 +22,7 @@ struct Market { /// @notice Authorization struct. /// @param authorizer Authorizer address. /// @param authorized Authorized address. -/// @param isAuthorized isAuthorized to set. +/// @param isAuthorized The authorization status to set. /// @param nonce Signature nonce. /// @param deadline Signature deadline. struct Authorization { @@ -213,7 +213,7 @@ interface IMorpho is IFlashLender { function setAuthorization(address authorized, bool newIsAuthorized) external; /// @notice Sets the authorization for `authorization.authorized` to manage `authorization.authorizer`'s positions. - /// @param authorization The Authorization struct. + /// @param authorization The `Authorization` struct. /// @param signature The signature. function setAuthorizationWithSig(Authorization calldata authorization, Signature calldata signature) external;