From 6f3319099308db24dc72b5a0532d127f94127886 Mon Sep 17 00:00:00 2001 From: Brendan Chou Date: Mon, 30 Jul 2018 11:36:44 -0700 Subject: [PATCH] add multiple signature types (#384) * add multiple signature types * fixes * fixes --- contracts/lib/TypedSignature.sol | 107 +++++++++++++++++++++++ contracts/margin/impl/BorrowShared.sol | 10 +-- contracts/testing/TestTypedSignature.sol | 36 ++++++++ test/helpers/Constants.js | 9 +- test/helpers/LoanHelper.js | 11 ++- test/lib/TestTypedSignature.js | 102 +++++++++++++++++++++ 6 files changed, 267 insertions(+), 8 deletions(-) create mode 100644 contracts/lib/TypedSignature.sol create mode 100644 contracts/testing/TestTypedSignature.sol create mode 100644 test/lib/TestTypedSignature.js diff --git a/contracts/lib/TypedSignature.sol b/contracts/lib/TypedSignature.sol new file mode 100644 index 00000000..e8e8f7e2 --- /dev/null +++ b/contracts/lib/TypedSignature.sol @@ -0,0 +1,107 @@ +/* + + Copyright 2018 dYdX Trading Inc. + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. + +*/ + +pragma solidity 0.4.24; +pragma experimental "v0.5.0"; + + +/** + * @title TypedSignature + * @author dYdX + * + * Allows for ecrecovery of signed hashes with three different prepended messages: + * 1) "" + * 2) "\x19Ethereum Signed Message:\n32" + * 3) "\x19Ethereum Signed Message:\n\x20" + */ +library TypedSignature { + + enum SignatureType { + INVALID, + ECRECOVER_NULL, + ECRECOVER_DEC, + ECRECOVER_HEX, + UNSUPPORTED + } + + // prepended message with the length of the signed hash in hexadecimal + bytes constant private PREPEND_HEX = "\x19Ethereum Signed Message:\n\x20"; + + // prepended message with the length of the signed hash in decimal + bytes constant private PREPEND_DEC = "\x19Ethereum Signed Message:\n32"; + + /** + * Gives the address of the signer of a hash. Allows for three common prepended strings. + * + * @param hash Hash that was signed (does not include prepended message) + * @param signatureWithType Type and ECDSA signature with structure: {1:type}{1:v}{32:r}{32:s} + * @return address of the signer of the hash + */ + function recover( + bytes32 hash, + bytes signatureWithType + ) + internal + pure + returns (address) + { + require( + signatureWithType.length == 66, + "SignatureValidator#validateSignature: invalid signature length" + ); + + uint8 rawSigType = uint8(signatureWithType[0]); + + require( + rawSigType > uint8(SignatureType.INVALID), + "SignatureValidator#validateSignature: invalid signature type" + ); + require( + rawSigType < uint8(SignatureType.UNSUPPORTED), + "SignatureValidator#validateSignature: unsupported signature type" + ); + + SignatureType sigType = SignatureType(rawSigType); + uint8 v = uint8(signatureWithType[1]); + bytes32 r; + bytes32 s; + + /* solium-disable-next-line security/no-inline-assembly */ + assembly { + r := mload(add(signatureWithType, 34)) + s := mload(add(signatureWithType, 66)) + } + + bytes32 signedHash; + if (sigType == SignatureType.ECRECOVER_DEC) { + signedHash = keccak256(abi.encodePacked(PREPEND_DEC, hash)); + } else if (sigType == SignatureType.ECRECOVER_HEX) { + signedHash = keccak256(abi.encodePacked(PREPEND_HEX, hash)); + } else { + assert(sigType == SignatureType.ECRECOVER_NULL); + signedHash = hash; + } + + return ecrecover( + signedHash, + v, + r, + s + ); + } +} diff --git a/contracts/margin/impl/BorrowShared.sol b/contracts/margin/impl/BorrowShared.sol index 18f550ac..40a15904 100644 --- a/contracts/margin/impl/BorrowShared.sol +++ b/contracts/margin/impl/BorrowShared.sol @@ -20,7 +20,6 @@ pragma solidity 0.4.24; pragma experimental "v0.5.0"; import { AddressUtils } from "zeppelin-solidity/contracts/AddressUtils.sol"; -import { ECRecovery } from "zeppelin-solidity/contracts/ECRecovery.sol"; import { Math } from "zeppelin-solidity/contracts/math/Math.sol"; import { SafeMath } from "zeppelin-solidity/contracts/math/SafeMath.sol"; import { MarginCommon } from "./MarginCommon.sol"; @@ -28,6 +27,7 @@ import { MarginState } from "./MarginState.sol"; import { TokenProxy } from "../TokenProxy.sol"; import { Vault } from "../Vault.sol"; import { MathHelpers } from "../../lib/MathHelpers.sol"; +import { TypedSignature } from "../../lib/TypedSignature.sol"; import { ExchangeWrapper } from "../interfaces/ExchangeWrapper.sol"; import { LoanOfferingVerifier } from "../interfaces/LoanOfferingVerifier.sol"; @@ -40,7 +40,6 @@ import { LoanOfferingVerifier } from "../interfaces/LoanOfferingVerifier.sol"; * Both use a Loan Offering and a DEX Order to open or increase a position. */ library BorrowShared { - using ECRecovery for bytes32; using SafeMath for uint256; // ============ Structs ============ @@ -97,10 +96,11 @@ library BorrowShared { if (AddressUtils.isContract(transaction.loanOffering.payer)) { getConsentFromSmartContractLender(transaction); } else { - bytes32 messageHash = transaction.loanOffering.loanHash.toEthSignedMessageHash(); require( - transaction.loanOffering.payer == - messageHash.recover(transaction.loanOffering.signature), + transaction.loanOffering.payer == TypedSignature.recover( + transaction.loanOffering.loanHash, + transaction.loanOffering.signature + ), "BorrowShared#validateTxPreSell: Invalid loan offering signature" ); } diff --git a/contracts/testing/TestTypedSignature.sol b/contracts/testing/TestTypedSignature.sol new file mode 100644 index 00000000..a894fbb0 --- /dev/null +++ b/contracts/testing/TestTypedSignature.sol @@ -0,0 +1,36 @@ +/* + + Copyright 2018 dYdX Trading Inc. + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. + +*/ + +pragma solidity 0.4.24; +pragma experimental "v0.5.0"; + +import { TypedSignature } from "../lib/TypedSignature.sol"; + + +contract TestTypedSignature { + function recover( + bytes32 hash, + bytes signatureWithType + ) + external + pure + returns (address) + { + return TypedSignature.recover(hash, signatureWithType); + } +} diff --git a/test/helpers/Constants.js b/test/helpers/Constants.js index 75f88979..b7385bd9 100644 --- a/test/helpers/Constants.js +++ b/test/helpers/Constants.js @@ -3,7 +3,14 @@ const web3Instance = new Web3(web3.currentProvider); const BigNumber = require('bignumber.js'); module.exports = { - ORDER_TYPE:{ + SIGNATURE_TYPE: { + INVALID_ZERO: 0, + INVALID_NONZERO: 15, + NUL: 1, + DEC: 2, + HEX: 3, + }, + ORDER_TYPE: { ZERO_EX: "zeroEx", KYBER: "kyber", DIRECT: "openDirectly" diff --git a/test/helpers/LoanHelper.js b/test/helpers/LoanHelper.js index b78de8c7..4d02c8ee 100644 --- a/test/helpers/LoanHelper.js +++ b/test/helpers/LoanHelper.js @@ -2,7 +2,7 @@ const BigNumber = require('bignumber.js'); const HeldToken = artifacts.require("TokenA"); const OwedToken = artifacts.require("TokenB"); const FeeToken = artifacts.require("TokenC"); -const { ADDRESSES, BIGNUMBERS, DEFAULT_SALT } = require('./Constants'); +const { ADDRESSES, BIGNUMBERS, DEFAULT_SALT, SIGNATURE_TYPE } = require('./Constants'); const Web3 = require('web3'); const Margin = artifacts.require("Margin"); const promisify = require("es6-promisify"); @@ -85,7 +85,14 @@ async function signLoanOffering(loanOffering) { ); const { v, r, s } = ethUtil.fromRpcSig(signature); - return ethUtil.bufferToHex(Buffer.concat([r, s, ethUtil.toBuffer(v)])); + return ethUtil.bufferToHex( + Buffer.concat([ + ethUtil.toBuffer(SIGNATURE_TYPE.DEC), + ethUtil.toBuffer(v), + r, + s, + ]) + ); } module.exports = { diff --git a/test/lib/TestTypedSignature.js b/test/lib/TestTypedSignature.js new file mode 100644 index 00000000..ebba42cf --- /dev/null +++ b/test/lib/TestTypedSignature.js @@ -0,0 +1,102 @@ +const chai = require('chai'); +const expect = chai.expect; +chai.use(require('chai-bignumber')()); +const BigNumber = require('bignumber.js'); +const promisify = require("es6-promisify"); +const ethUtil = require('ethereumjs-util'); +const Web3 = require('web3'); +const web3Instance = new Web3(web3.currentProvider); + +const TestTypedSignature = artifacts.require("TestTypedSignature"); +const { BIGNUMBERS, BYTES32, SIGNATURE_TYPE } = require('../helpers/Constants'); +const { expectThrow } = require('../helpers/ExpectHelper'); + +contract('TestTypedSignature', accounts => { + let contract; + const rawKey = "43f2ee33c522046e80b67e96ceb84a05b60b9434b0ee2e3ae4b1311b9f5dcc46"; + const privateKey = new Buffer(rawKey, "hex"); + const signer = "0x" + ethUtil.privateToAddress(privateKey).toString("hex"); + const hash = BYTES32.TEST[0]; + const PROPER_SIG_LENGTH = 66; + + before(async () => { + contract = await TestTypedSignature.new(); + }); + + describe('INVALID', () => { + it("fails when invalid type (zero)", async () => { + const signature = await promisify(web3Instance.eth.sign)(hash, accounts[0]); + const { v, r, s } = ethUtil.fromRpcSig(signature); + const signatureWithType = getSignature(SIGNATURE_TYPE.INVALID_ZERO, v, r, s); + await expectThrow(contract.recover(hash, signatureWithType)); + }); + + it("fails when invalid type (nonzero)", async () => { + const signature = await promisify(web3Instance.eth.sign)(hash, accounts[0]); + const { v, r, s } = ethUtil.fromRpcSig(signature); + const signatureWithType = getSignature(SIGNATURE_TYPE.INVALID_NONZERO, v, r, s); + await expectThrow(contract.recover(hash, signatureWithType)); + }); + + it("fails when too short", async () => { + const n = Buffer.from("12345678123456781234567812345678"); + const tooShort = Buffer.concat([n, n]); + const signatureWithType = ethUtil.bufferToHex(tooShort); + expect(tooShort.length).to.be.lt(PROPER_SIG_LENGTH); + await expectThrow(contract.recover(hash, signatureWithType)); + }); + + it("fails when too long", async () => { + const n = Buffer.from("12345678123456781234567812345678"); + const tooLong = Buffer.concat([n, n, n, n]); + const signatureWithType = ethUtil.bufferToHex(tooLong); + expect(tooLong.length).to.be.gt(PROPER_SIG_LENGTH); + await expectThrow(contract.recover(hash, signatureWithType)); + }); + }); + + describe('ECRECOVERY_NUL', () => { + it("returns the correct signer", async () => { + const ecSignature = ethUtil.ecsign(ethUtil.toBuffer(hash), privateKey); + const { v, r, s } = ecSignature; + const signatureWithType = getSignature(SIGNATURE_TYPE.NUL, v, r, s); + const retVal = await contract.recover.call(hash, signatureWithType); + expect(retVal).to.equal(signer); + }); + }); + + describe('ECRECOVERY_DEC', () => { + it("returns the correct signer", async () => { + const signer = accounts[0]; + const signature = await promisify(web3Instance.eth.sign)(hash, accounts[0]); + const { v, r, s } = ethUtil.fromRpcSig(signature); + + const signatureWithType = getSignature(SIGNATURE_TYPE.DEC, v, r, s); + const retVal = await contract.recover(hash, signatureWithType); + expect(retVal).to.equal(signer); + }); + }); + + describe('ECRECOVERY_HEX', () => { + it("returns the correct signer", async () => { + const packed = "\x19Ethereum Signed Message:\n\x20" + ethUtil.toBuffer(hash); + const hash2 = "0x" + ethUtil.sha3(Buffer.from(packed, 32)).toString("hex"); + const ecSignature = ethUtil.ecsign(ethUtil.toBuffer(hash2), privateKey); + const { v, r, s } = ecSignature; + const signatureWithType = getSignature(SIGNATURE_TYPE.HEX, v, r, s); + const retVal = await contract.recover.call(hash, signatureWithType); + expect(retVal).to.equal(signer); + }); + }); +}); + +function getSignature(type, v, r, s) { + return ethUtil.bufferToHex( + Buffer.concat([ + ethUtil.toBuffer(type), + ethUtil.toBuffer(v), + r, + s, + ]) + ); +}