crowetic/protocol
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Merge branch 'development' into fix/broken-validator-signatures

Browse Source
This commit is contained in:
Jan-Gerrit Harms
2019-08-20 09:30:21 +02:00
parent b2592d1cc2 90cd364780
commit 3a46f1a27a
49 changed files with 1024 additions and 952 deletions
Download Patch File Download Diff File Expand all files Collapse all files

10
contracts/exchange/contracts/examples/Validator.sol
View File

@@ -21,7 +21,7 @@ pragma solidity ^0.5.5;
import "../src/interfaces/IValidator.sol";
contract Validator is
contract Validator is
IValidator
{
@@ -39,7 +39,7 @@ contract Validator is
/// @param hash Message hash that is signed.
/// @param signerAddress Address that should have signed the given hash.
/// @param signature Proof of signing.
/// @return Validity of signature.
/// @return Returns a known magic value if the signature is valid.
// solhint-disable no-unused-vars
function isValidSignature(
bytes32 hash,
@@ -48,9 +48,11 @@ contract Validator is
)
external
view
returns (bool isValid)
returns (bytes4)
{
return (signerAddress == VALID_SIGNER);
require(signerAddress == VALID_SIGNER, "INVALID_SIGNER");
bytes4 magicValue = bytes4(keccak256("isValidValidatorSignature(address,bytes32,address,bytes)"));
return magicValue;
}
// solhint-enable no-unused-vars
}

11
contracts/exchange/contracts/examples/Wallet.sol
View File

@@ -22,7 +22,7 @@ import "../src/interfaces/IWallet.sol";
import "@0x/contracts-utils/contracts/src/LibBytes.sol";
contract Wallet is
contract Wallet is
IWallet
{
using LibBytes for bytes;
@@ -41,14 +41,14 @@ contract Wallet is
/// The signer must match the owner of this wallet.
/// @param hash Message hash that is signed.
/// @param eip712Signature Proof of signing.
/// @return Validity of signature.
/// @return Returns a known magic value if the signature is valid.
function isValidSignature(
bytes32 hash,
bytes calldata eip712Signature
)
external
view
returns (bool isValid)
returns (bytes4)
{
require(
eip712Signature.length == 65,
@@ -59,7 +59,8 @@ contract Wallet is
bytes32 r = eip712Signature.readBytes32(1);
bytes32 s = eip712Signature.readBytes32(33);
address recoveredAddress = ecrecover(hash, v, r, s);
isValid = WALLET_OWNER == recoveredAddress;
return isValid;
require(WALLET_OWNER == recoveredAddress, "INVALID_SIGNATURE");
bytes4 magicValue = bytes4(keccak256("isValidWalletSignature(bytes32,address,bytes)"));
return magicValue;
}
}

6
contracts/exchange/contracts/examples/Whitelist.sol
View File

@@ -73,10 +73,12 @@ contract Whitelist is
)
external
view
returns (bool isValid)
returns (bytes4)
{
// solhint-disable-next-line avoid-tx-origin
return signerAddress == tx.origin;
require(signerAddress == tx.origin, "INVALID_SIGNER");
bytes4 magicValue = bytes4(keccak256("isValidValidatorSignature(address,bytes32,address,bytes)"));
return magicValue;
}
// solhint-enable no-unused-vars

60
contracts/exchange/contracts/src/MixinSignatureValidator.sol
View File

@@ -32,7 +32,7 @@ contract MixinSignatureValidator is
MTransactions
{
using LibBytes for bytes;
// Mapping of hash => signer => signed
mapping (bytes32 => mapping (address => bool)) public preSigned;
@@ -197,7 +197,7 @@ contract MixinSignatureValidator is
} else if (signatureType == SignatureType.Validator) {
// Pop last 20 bytes off of signature byte array.
address validatorAddress = signature.popLast20Bytes();
// Ensure signer has approved validator.
if (!allowedValidators[signerAddress][validatorAddress]) {
return false;
@@ -224,7 +224,8 @@ contract MixinSignatureValidator is
revert("SIGNATURE_UNSUPPORTED");
}
/// @dev Verifies signature using logic defined by Wallet contract.
/// @dev Verifies signature using logic defined by Wallet contract. Wallet contract
/// must return `bytes4(keccak256("isValidWalletSignature(bytes32,address,bytes)"))`
/// @param hash Any 32 byte hash.
/// @param walletAddress Address that should have signed the given hash
/// and defines its own signature verification method.
@@ -244,7 +245,19 @@ contract MixinSignatureValidator is
hash,
signature
);
// bytes4 0xb0671381
bytes32 magicValue = bytes32(bytes4(keccak256("isValidWalletSignature(bytes32,address,bytes)")));
assembly {
// extcodesize added as an extra safety measure
if iszero(extcodesize(walletAddress)) {
// Revert with `Error("WALLET_ERROR")`
mstore(0, 0x08c379a000000000000000000000000000000000000000000000000000000000)
mstore(32, 0x0000002000000000000000000000000000000000000000000000000000000000)
mstore(64, 0x0000000c57414c4c45545f4552524f5200000000000000000000000000000000)
mstore(96, 0)
revert(0, 100)
}
let cdStart := add(callData, 32)
let success := staticcall(
gas, // forward all gas
@@ -255,6 +268,15 @@ contract MixinSignatureValidator is
32 // output size is 32 bytes
)
if iszero(eq(returndatasize(), 32)) {
// Revert with `Error("WALLET_ERROR")`
mstore(0, 0x08c379a000000000000000000000000000000000000000000000000000000000)
mstore(32, 0x0000002000000000000000000000000000000000000000000000000000000000)
mstore(64, 0x0000000c57414c4c45545f4552524f5200000000000000000000000000000000)
mstore(96, 0)
revert(0, 100)
}
switch success
case 0 {
// Revert with `Error("WALLET_ERROR")`
@@ -266,13 +288,17 @@ contract MixinSignatureValidator is
}
case 1 {
// Signature is valid if call did not revert and returned true
isValid := mload(cdStart)
isValid := eq(
and(mload(cdStart), 0xffffffff00000000000000000000000000000000000000000000000000000000),
and(magicValue, 0xffffffff00000000000000000000000000000000000000000000000000000000)
)
}
}
return isValid;
}
/// @dev Verifies signature using logic defined by Validator contract.
/// Validator must return `bytes4(keccak256("isValidValidatorSignature(address,bytes32,address,bytes)"))`
/// @param validatorAddress Address of validator contract.
/// @param hash Any 32 byte hash.
/// @param signerAddress Address that should have signed the given hash.
@@ -294,7 +320,19 @@ contract MixinSignatureValidator is
signerAddress,
signature
);
// bytes4 0x42b38674
bytes32 magicValue = bytes32(bytes4(keccak256("isValidValidatorSignature(address,bytes32,address,bytes)")));
assembly {
// extcodesize added as an extra safety measure
if iszero(extcodesize(validatorAddress)) {
// Revert with `Error("VALIDATOR_ERROR")`
mstore(0, 0x08c379a000000000000000000000000000000000000000000000000000000000)
mstore(32, 0x0000002000000000000000000000000000000000000000000000000000000000)
mstore(64, 0x0000000f56414c494441544f525f4552524f5200000000000000000000000000)
mstore(96, 0)
revert(0, 100)
}
let cdStart := add(callData, 32)
let success := staticcall(
gas, // forward all gas
@@ -305,6 +343,15 @@ contract MixinSignatureValidator is
32 // output size is 32 bytes
)
if iszero(eq(returndatasize(), 32)) {
// Revert with `Error("VALIDATOR_ERROR")`
mstore(0, 0x08c379a000000000000000000000000000000000000000000000000000000000)
mstore(32, 0x0000002000000000000000000000000000000000000000000000000000000000)
mstore(64, 0x0000000f56414c494441544f525f4552524f5200000000000000000000000000)
mstore(96, 0)
revert(0, 100)
}
switch success
case 0 {
// Revert with `Error("VALIDATOR_ERROR")`
@@ -316,7 +363,10 @@ contract MixinSignatureValidator is
}
case 1 {
// Signature is valid if call did not revert and returned true
isValid := mload(cdStart)
isValid := eq(
and(mload(cdStart), 0xffffffff00000000000000000000000000000000000000000000000000000000),
and(magicValue, 0xffffffff00000000000000000000000000000000000000000000000000000000)
)
}
}
return isValid;

5
contracts/exchange/contracts/src/interfaces/IValidator.sol
View File

@@ -25,7 +25,8 @@ contract IValidator {
/// @param hash Message hash that is signed.
/// @param signerAddress Address that should have signed the given hash.
/// @param signature Proof of signing.
/// @return Validity of order signature.
/// @return Magic bytes4 value if the signature is valid.
/// Magic value is bytes4(keccak256("isValidValidatorSignature(address,bytes32,address,bytes)"))
function isValidSignature(
bytes32 hash,
address signerAddress,
@@ -33,5 +34,5 @@ contract IValidator {
)
external
view
returns (bool isValid);
returns (bytes4);
}

5
contracts/exchange/contracts/src/interfaces/IWallet.sol
View File

@@ -24,12 +24,13 @@ contract IWallet {
/// @dev Verifies that a signature is valid.
/// @param hash Message hash that is signed.
/// @param signature Proof of signing.
/// @return Validity of order signature.
/// @return Magic bytes4 value if the signature is valid.
/// Magic value is bytes4(keccak256("isValidWalletSignature(bytes32,address,bytes)"))
function isValidSignature(
bytes32 hash,
bytes calldata signature
)
external
view
returns (bool isValid);
returns (bytes4);
}

10
contracts/exchange/test/signature_validator.ts
View File

@@ -313,7 +313,7 @@ describe('MixinSignatureValidator', () => {
expect(isValidSignature).to.be.true();
});
it('should return false when SignatureType=Wallet and signature is invalid', async () => {
it('should revert when SignatureType=Wallet and signature is invalid', async () => {
// Create EIP712 signature using a private key that does not belong to the wallet owner.
const orderHashHex = orderHashUtils.getOrderHashHex(signedOrder);
const orderHashBuffer = ethUtil.toBuffer(orderHashHex);
@@ -328,12 +328,10 @@ describe('MixinSignatureValidator', () => {
]);
const signatureHex = ethUtil.bufferToHex(signature);
// Validate signature
const isValidSignature = await signatureValidator.publicIsValidSignature.callAsync(
orderHashHex,
testWallet.address,
signatureHex,
return expectContractCallFailedAsync(
signatureValidator.publicIsValidSignature.callAsync(orderHashHex, testWallet.address, signatureHex),
RevertReason.WalletError,
);
expect(isValidSignature).to.be.false();
});
it('should revert when `isValidSignature` attempts to update state and SignatureType=Wallet', async () => {