diff --git a/contracts/test/EIP712Test.sol b/contracts/test/EIP712Test.sol index fe54065f5..e34dcb698 100644 --- a/contracts/test/EIP712Test.sol +++ b/contracts/test/EIP712Test.sol @@ -1,7 +1,7 @@ /** * SPDX-License-Identifier: MIT * - * Copyright (c) 2018 zOS Global Limited. + * Copyright (c) 2018 - 2023 CENTRE SECZ * * Permission is hereby granted, free of charge, to any person obtaining a copy * of this software and associated documentation files (the "Software"), to deal @@ -34,14 +34,4 @@ contract EIP712Test { { return EIP712.makeDomainSeparator(name, version); } - - function recover( - bytes32 domainSeparator, - uint8 v, - bytes32 r, - bytes32 s, - bytes calldata typeHashAndData - ) external pure returns (address) { - return EIP712.recover(domainSeparator, v, r, s, typeHashAndData); - } } diff --git a/contracts/util/EIP712.sol b/contracts/util/EIP712.sol index ac0a69c2d..db89b05a5 100644 --- a/contracts/util/EIP712.sol +++ b/contracts/util/EIP712.sol @@ -1,7 +1,7 @@ /** * SPDX-License-Identifier: MIT * - * Copyright (c) 2018-2020 CENTRE SECZ + * Copyright (c) 2018-2023 CENTRE SECZ * * Permission is hereby granted, free of charge, to any person obtaining a copy * of this software and associated documentation files (the "Software"), to deal @@ -73,30 +73,4 @@ library EIP712 { } return makeDomainSeparator(name, version, chainId); } - - /** - * @notice Recover signer's address from a EIP712 signature - * @param domainSeparator Domain separator - * @param v v of the signature - * @param r r of the signature - * @param s s of the signature - * @param typeHashAndData Type hash concatenated with data - * @return Signer's address - */ - function recover( - bytes32 domainSeparator, - uint8 v, - bytes32 r, - bytes32 s, - bytes memory typeHashAndData - ) internal pure returns (address) { - bytes32 digest = keccak256( - abi.encodePacked( - "\x19\x01", - domainSeparator, - keccak256(typeHashAndData) - ) - ); - return ECRecover.recover(digest, v, r, s); - } } diff --git a/contracts/v2/EIP2612.sol b/contracts/v2/EIP2612.sol index 9a9b90a75..83ef8804e 100644 --- a/contracts/v2/EIP2612.sol +++ b/contracts/v2/EIP2612.sol @@ -1,7 +1,7 @@ /** * SPDX-License-Identifier: MIT * - * Copyright (c) 2018-2020 CENTRE SECZ + * Copyright (c) 2018-2023 CENTRE SECZ * * Permission is hereby granted, free of charge, to any person obtaining a copy * of this software and associated documentation files (the "Software"), to deal @@ -26,7 +26,8 @@ pragma solidity 0.6.12; import { AbstractFiatTokenV2 } from "./AbstractFiatTokenV2.sol"; import { EIP712Domain } from "./EIP712Domain.sol"; -import { EIP712 } from "../util/EIP712.sol"; +import { MessageHashUtils } from "../util/MessageHashUtils.sol"; +import { SignatureChecker } from "../util/SignatureChecker.sol"; /** * @title EIP-2612 @@ -66,19 +67,47 @@ abstract contract EIP2612 is AbstractFiatTokenV2, EIP712Domain { uint8 v, bytes32 r, bytes32 s + ) internal { + _permit(owner, spender, value, deadline, abi.encodePacked(r, s, v)); + } + + /** + * @notice Verify a signed approval permit and execute if valid + * @dev EOA wallet signatures should be packed in the order of r, s, v. + * @param owner Token owner's address (Authorizer) + * @param spender Spender's address + * @param value Amount of allowance + * @param deadline The time at which this expires (unix time) + * @param signature Signature byte array signed by an EOA wallet or a contract wallet + */ + function _permit( + address owner, + address spender, + uint256 value, + uint256 deadline, + bytes memory signature ) internal { require(deadline >= now, "FiatTokenV2: permit is expired"); - bytes memory data = abi.encode( - PERMIT_TYPEHASH, - owner, - spender, - value, - _permitNonces[owner]++, - deadline + bytes32 typedDataHash = MessageHashUtils.toTypedDataHash( + _domainSeparator(), + keccak256( + abi.encode( + PERMIT_TYPEHASH, + owner, + spender, + value, + _permitNonces[owner]++, + deadline + ) + ) ); require( - EIP712.recover(_domainSeparator(), v, r, s, data) == owner, + SignatureChecker.isValidSignatureNow( + owner, + typedDataHash, + signature + ), "EIP2612: invalid signature" ); diff --git a/contracts/v2/EIP3009.sol b/contracts/v2/EIP3009.sol index eeaa3c13d..94bb68589 100644 --- a/contracts/v2/EIP3009.sol +++ b/contracts/v2/EIP3009.sol @@ -1,7 +1,7 @@ /** * SPDX-License-Identifier: MIT * - * Copyright (c) 2018-2020 CENTRE SECZ + * Copyright (c) 2018-2023 CENTRE SECZ * * Permission is hereby granted, free of charge, to any person obtaining a copy * of this software and associated documentation files (the "Software"), to deal @@ -26,7 +26,8 @@ pragma solidity 0.6.12; import { AbstractFiatTokenV2 } from "./AbstractFiatTokenV2.sol"; import { EIP712Domain } from "./EIP712Domain.sol"; -import { EIP712 } from "../util/EIP712.sol"; +import { SignatureChecker } from "../util/SignatureChecker.sol"; +import { MessageHashUtils } from "../util/MessageHashUtils.sol"; /** * @title EIP-3009 @@ -97,20 +98,52 @@ abstract contract EIP3009 is AbstractFiatTokenV2, EIP712Domain { bytes32 r, bytes32 s ) internal { - _requireValidAuthorization(from, nonce, validAfter, validBefore); - - bytes memory data = abi.encode( - TRANSFER_WITH_AUTHORIZATION_TYPEHASH, + _transferWithAuthorization( from, to, value, validAfter, validBefore, - nonce + nonce, + abi.encodePacked(r, s, v) ); - require( - EIP712.recover(_domainSeparator(), v, r, s, data) == from, - "FiatTokenV2: invalid signature" + } + + /** + * @notice Execute a transfer with a signed authorization + * @dev EOA wallet signatures should be packed in the order of r, s, v. + * @param from Payer's address (Authorizer) + * @param to Payee's address + * @param value Amount to be transferred + * @param validAfter The time after which this is valid (unix time) + * @param validBefore The time before which this is valid (unix time) + * @param nonce Unique nonce + * @param signature Signature byte array produced by an EOA wallet or a contract wallet + */ + function _transferWithAuthorization( + address from, + address to, + uint256 value, + uint256 validAfter, + uint256 validBefore, + bytes32 nonce, + bytes memory signature + ) internal { + _requireValidAuthorization(from, nonce, validAfter, validBefore); + _requireValidSignature( + from, + keccak256( + abi.encode( + TRANSFER_WITH_AUTHORIZATION_TYPEHASH, + from, + to, + value, + validAfter, + validBefore, + nonce + ) + ), + signature ); _markAuthorizationAsUsed(from, nonce); @@ -142,21 +175,55 @@ abstract contract EIP3009 is AbstractFiatTokenV2, EIP712Domain { bytes32 r, bytes32 s ) internal { - require(to == msg.sender, "FiatTokenV2: caller must be the payee"); - _requireValidAuthorization(from, nonce, validAfter, validBefore); - - bytes memory data = abi.encode( - RECEIVE_WITH_AUTHORIZATION_TYPEHASH, + _receiveWithAuthorization( from, to, value, validAfter, validBefore, - nonce + nonce, + abi.encodePacked(r, s, v) ); - require( - EIP712.recover(_domainSeparator(), v, r, s, data) == from, - "FiatTokenV2: invalid signature" + } + + /** + * @notice Receive a transfer with a signed authorization from the payer + * @dev This has an additional check to ensure that the payee's address + * matches the caller of this function to prevent front-running attacks. + * EOA wallet signatures should be packed in the order of r, s, v. + * @param from Payer's address (Authorizer) + * @param to Payee's address + * @param value Amount to be transferred + * @param validAfter The time after which this is valid (unix time) + * @param validBefore The time before which this is valid (unix time) + * @param nonce Unique nonce + * @param signature Signature byte array produced by an EOA wallet or a contract wallet + */ + function _receiveWithAuthorization( + address from, + address to, + uint256 value, + uint256 validAfter, + uint256 validBefore, + bytes32 nonce, + bytes memory signature + ) internal { + require(to == msg.sender, "FiatTokenV2: caller must be the payee"); + _requireValidAuthorization(from, nonce, validAfter, validBefore); + _requireValidSignature( + from, + keccak256( + abi.encode( + RECEIVE_WITH_AUTHORIZATION_TYPEHASH, + from, + to, + value, + validAfter, + validBefore, + nonce + ) + ), + signature ); _markAuthorizationAsUsed(from, nonce); @@ -178,22 +245,55 @@ abstract contract EIP3009 is AbstractFiatTokenV2, EIP712Domain { bytes32 r, bytes32 s ) internal { - _requireUnusedAuthorization(authorizer, nonce); + _cancelAuthorization(authorizer, nonce, abi.encodePacked(r, s, v)); + } - bytes memory data = abi.encode( - CANCEL_AUTHORIZATION_TYPEHASH, + /** + * @notice Attempt to cancel an authorization + * @dev EOA wallet signatures should be packed in the order of r, s, v. + * @param authorizer Authorizer's address + * @param nonce Nonce of the authorization + * @param signature Signature byte array produced by an EOA wallet or a contract wallet + */ + function _cancelAuthorization( + address authorizer, + bytes32 nonce, + bytes memory signature + ) internal { + _requireUnusedAuthorization(authorizer, nonce); + _requireValidSignature( authorizer, - nonce - ); - require( - EIP712.recover(_domainSeparator(), v, r, s, data) == authorizer, - "FiatTokenV2: invalid signature" + keccak256( + abi.encode(CANCEL_AUTHORIZATION_TYPEHASH, authorizer, nonce) + ), + signature ); _authorizationStates[authorizer][nonce] = true; emit AuthorizationCanceled(authorizer, nonce); } + /** + * @notice Validates that signature against input data struct + * @param signer Signer's address + * @param dataHash Hash of encoded data struct + * @param signature Signature byte array produced by an EOA wallet or a contract wallet + */ + function _requireValidSignature( + address signer, + bytes32 dataHash, + bytes memory signature + ) private view { + require( + SignatureChecker.isValidSignatureNow( + signer, + MessageHashUtils.toTypedDataHash(_domainSeparator(), dataHash), + signature + ), + "FiatTokenV2: invalid signature" + ); + } + /** * @notice Check that an authorization is unused * @param authorizer Authorizer's address diff --git a/migrations/3_deploy_v2.js b/migrations/3_deploy_v2.js index 39de3e1b3..656cba8e9 100644 --- a/migrations/3_deploy_v2.js +++ b/migrations/3_deploy_v2.js @@ -5,6 +5,7 @@ const some = require("lodash/some"); const FiatTokenV2 = artifacts.require("FiatTokenV2"); const FiatTokenProxy = artifacts.require("FiatTokenProxy"); const FiatTokenUtil = artifacts.require("FiatTokenUtil"); +const SignatureChecker = artifacts.require("SignatureChecker"); const THROWAWAY_ADDRESS = "0x0000000000000000000000000000000000000001"; @@ -25,6 +26,10 @@ module.exports = async (deployer, network) => { console.log(`FiatTokenProxy: ${proxyContractAddress}`); + console.log("Deploying and linking SignatureChecker library contract..."); + await deployer.deploy(SignatureChecker); + await deployer.link(SignatureChecker, FiatTokenV2); + console.log("Deploying FiatTokenV2 implementation contract..."); await deployer.deploy(FiatTokenV2); diff --git a/migrations/5_deploy_v2_1.js b/migrations/5_deploy_v2_1.js index ac3c00ce2..dcb804d3a 100644 --- a/migrations/5_deploy_v2_1.js +++ b/migrations/5_deploy_v2_1.js @@ -4,6 +4,7 @@ const some = require("lodash/some"); const FiatTokenV2_1 = artifacts.require("FiatTokenV2_1"); const FiatTokenProxy = artifacts.require("FiatTokenProxy"); +const SignatureChecker = artifacts.require("SignatureChecker"); const THROWAWAY_ADDRESS = "0x0000000000000000000000000000000000000001"; @@ -24,6 +25,10 @@ module.exports = async (deployer, network) => { console.log(`FiatTokenProxy: ${proxyContractAddress}`); + console.log("Deploying and linking SignatureChecker library contract..."); + await deployer.deploy(SignatureChecker); + await deployer.link(SignatureChecker, FiatTokenV2_1); + console.log("Deploying FiatTokenV2_1 implementation contract..."); await deployer.deploy(FiatTokenV2_1); diff --git a/migrations/7_deploy_v2_2.js b/migrations/7_deploy_v2_2.js index c6ba76bdf..1fd310fd2 100644 --- a/migrations/7_deploy_v2_2.js +++ b/migrations/7_deploy_v2_2.js @@ -4,6 +4,7 @@ const some = require("lodash/some"); const FiatTokenV2_2 = artifacts.require("FiatTokenV2_2"); const FiatTokenProxy = artifacts.require("FiatTokenProxy"); +const SignatureChecker = artifacts.require("SignatureChecker"); const THROWAWAY_ADDRESS = "0x0000000000000000000000000000000000000001"; @@ -24,6 +25,10 @@ module.exports = async (deployer, network) => { console.log(`FiatTokenProxy: ${proxyContractAddress}`); + console.log("Deploying and linking SignatureChecker library contract..."); + await deployer.deploy(SignatureChecker); + await deployer.link(SignatureChecker, FiatTokenV2_2); + console.log("Deploying FiatTokenV2_2 implementation contract..."); await deployer.deploy(FiatTokenV2_2); diff --git a/test/util/EIP712Test.ts b/test/util/EIP712Test.ts index ecef12d18..a3af95641 100644 --- a/test/util/EIP712Test.ts +++ b/test/util/EIP712Test.ts @@ -1,9 +1,7 @@ -import crypto from "crypto"; import { Eip712TestInstance } from "../../@types/generated"; import { wordlist } from "ethereum-cryptography/bip39/wordlists/english"; import sampleSize from "lodash/sampleSize"; -import { ACCOUNTS_AND_KEYS } from "../helpers/constants"; -import { prepend0x, strip0x, ecSign, makeDomainSeparator } from "../helpers"; +import { makeDomainSeparator } from "../helpers"; const EIP712Test = artifacts.require("EIP712Test"); @@ -39,26 +37,4 @@ contract("EIP712", (_accounts) => { ).to.equal(domainSeparator); }); }); - - describe("recover", () => { - it("recovers the signer's address from signed data", async () => { - const randomAccount = - ACCOUNTS_AND_KEYS[Math.floor(Math.random() * ACCOUNTS_AND_KEYS.length)]; - const randomData = prepend0x(crypto.randomBytes(256).toString("hex")); - const eip712Data = prepend0x( - "1901" + - strip0x(domainSeparator) + - strip0x(web3.utils.keccak256(randomData)) - ); - - const { v, r, s } = ecSign( - web3.utils.keccak256(eip712Data), - randomAccount.key - ); - - expect( - await eip712.recover(domainSeparator, v, r, s, randomData) - ).to.equal(randomAccount.address); - }); - }); }); diff --git a/test/v2/MockFiatTokenWithEditableChainId.test.ts b/test/v2/MockFiatTokenWithEditableChainId.test.ts index 20a1f1748..31b58f4f0 100644 --- a/test/v2/MockFiatTokenWithEditableChainId.test.ts +++ b/test/v2/MockFiatTokenWithEditableChainId.test.ts @@ -1,6 +1,7 @@ import { MockFiatTokenWithEditableChainIdInstance } from "../../@types/generated"; import { makeDomainSeparator } from "../helpers"; +const SignatureChecker = artifacts.require("SignatureChecker"); const MockFiatTokenWithEditableChainId = artifacts.require( "MockFiatTokenWithEditableChainId" ); @@ -13,6 +14,8 @@ contract("MockFiatTokenWithEditableChainId", (accounts) => { const version = "2"; beforeEach(async () => { + await SignatureChecker.new(); + MockFiatTokenWithEditableChainId.link(SignatureChecker); fiatToken = await MockFiatTokenWithEditableChainId.new(); await fiatToken.initialize(