Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Signature frontrun alternative fix #271

Closed
wants to merge 10 commits into from
30 changes: 12 additions & 18 deletions src/Morpho.sol
Original file line number Diff line number Diff line change
Expand Up @@ -382,28 +382,22 @@ contract Morpho is IMorpho {

/// @inheritdoc IMorpho
/// @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);

require(signatory != address(0) && authorizer == signatory, ErrorsLib.INVALID_SIGNATURE);

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);
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
);
}
}

function _isSenderAuthorized(address user) internal view returns (bool) {
Expand Down
28 changes: 17 additions & 11 deletions src/interfaces/IMorpho.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -199,18 +213,10 @@ interface IMorpho is IFlashLender {
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.
/// @param authorization The authorization struct.
/// @param signature The signature.
/// @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;
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);
Expand Down
61 changes: 42 additions & 19 deletions test/forge/Morpho.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -853,32 +853,55 @@ 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);

// 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, sig);

assertEq(morpho.isAuthorized(authorization.authorizer, authorization.authorized), authorization.isAuthorized);
assertEq(morpho.nonce(authorization.authorizer), 1);
}

function testAuthorizationWithSigWrongSig(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 = 1;
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), false);
assertEq(morpho.nonce(authorization.authorizer), 0);
}

function testAuthorizationWithSigWrongNonce(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 = 1;

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 amount) public {
Expand Down
10 changes: 1 addition & 9 deletions test/forge/helpers/SigUtils.sol
Original file line number Diff line number Diff line change
@@ -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
Expand Down