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

Use last byte of signature as signature type

Browse Source
This commit is contained in:
Amir Bandeali
2018-05-21 15:59:58 -07:00
parent 6d462fc961
commit 822e319efe
8 changed files with 106 additions and 109 deletions
Download Patch File Download Diff File Expand all files Collapse all files

58
packages/contracts/src/contracts/current/protocol/Exchange/MixinSignatureValidator.sol
View File

@@ -81,7 +81,9 @@ contract MixinSignatureValidator is
signature.length >= 1,
INVALID_SIGNATURE_LENGTH
);
SignatureType signatureType = SignatureType(uint8(signature[0]));
// Pop last byte off of
SignatureType signatureType = SignatureType(uint8(popByte(signature)));
// Variables are not scoped in Solidity.
uint8 v;
@@ -104,7 +106,7 @@ contract MixinSignatureValidator is
// a correctly formatted but incorrect signature.
} else if (signatureType == SignatureType.Invalid) {
require(
signature.length == 1,
signature.length == 0,
INVALID_SIGNATURE_LENGTH
);
isValid = false;
@@ -113,12 +115,12 @@ contract MixinSignatureValidator is
// Signature using EIP712
} else if (signatureType == SignatureType.EIP712) {
require(
signature.length == 66,
signature.length == 65,
INVALID_SIGNATURE_LENGTH
);
v = uint8(signature[1]);
r = readBytes32(signature, 2);
s = readBytes32(signature, 34);
v = uint8(signature[0]);
r = readBytes32(signature, 1);
s = readBytes32(signature, 33);
recovered = ecrecover(hash, v, r, s);
isValid = signer == recovered;
return isValid;
@@ -126,12 +128,12 @@ contract MixinSignatureValidator is
// Signed using web3.eth_sign
} else if (signatureType == SignatureType.Ecrecover) {
require(
signature.length == 66,
signature.length == 65,
INVALID_SIGNATURE_LENGTH
);
v = uint8(signature[1]);
r = readBytes32(signature, 2);
s = readBytes32(signature, 34);
v = uint8(signature[0]);
r = readBytes32(signature, 1);
s = readBytes32(signature, 33);
recovered = ecrecover(
keccak256("\x19Ethereum Signed Message:\n32", hash),
v,
@@ -151,7 +153,7 @@ contract MixinSignatureValidator is
// submit using `Caller`. Having `Caller` allows this flexibility.
} else if (signatureType == SignatureType.Caller) {
require(
signature.length == 1,
signature.length == 0,
INVALID_SIGNATURE_LENGTH
);
isValid = signer == msg.sender;
@@ -160,37 +162,25 @@ contract MixinSignatureValidator is
// Signature verified by signer contract.
// If used with an order, the maker of the order is the signer contract.
} else if (signatureType == SignatureType.Signer) {
// Pass in signature without signature type.
bytes memory signatureWithoutType = deepCopyBytes(
signature,
1,
signature.length - 1
);
isValid = ISigner(signer).isValidSignature(hash, signatureWithoutType);
isValid = ISigner(signer).isValidSignature(hash, signature);
return isValid;
// Signature verified by validator contract.
// If used with an order, the maker of the order can still be an EOA.
// A signature using this type should be encoded as:
// | Offset | Length | Contents |
// | 0x00 | 1 | Signature type is always "\x06" |
// | 0x01 | 20 | Address of validator contract |
// | 0x15 | ** | Signature to validate |
// | Offset | Length | Contents |
// | 0x00 | x | Signature to validate |
// | 0x00 + x | 20 | Address of validator contract |
// | 0x14 + x | 1 | Signature type is always "\x06" |
} else if (signatureType == SignatureType.Validator) {
address validator = readAddress(signature, 1);
address validator = popAddress(signature);
if (!allowedValidators[signer][validator]) {
return false;
}
// Pass in signature without type or validator address.
bytes memory signatureWithoutTypeOrAddress = deepCopyBytes(
signature,
21,
signature.length - 21
);
isValid = IValidator(validator).isValidSignature(
hash,
signer,
signatureWithoutTypeOrAddress
signature
);
return isValid;
@@ -209,12 +199,12 @@ contract MixinSignatureValidator is
// https://github.com/trezor/trezor-mcu/blob/master/firmware/crypto.c#L36
} else if (signatureType == SignatureType.Trezor) {
require(
signature.length == 66,
signature.length == 65,
INVALID_SIGNATURE_LENGTH
);
v = uint8(signature[1]);
r = readBytes32(signature, 2);
s = readBytes32(signature, 34);
v = uint8(signature[0]);
r = readBytes32(signature, 1);
s = readBytes32(signature, 33);
recovered = ecrecover(
keccak256("\x19Ethereum Signed Message:\n\x41", hash),
v,

31
packages/contracts/src/contracts/current/test/TestLibBytes/TestLibBytes.sol
View File

@@ -25,21 +25,26 @@ contract TestLibBytes is
LibBytes
{
/// @dev Performs a deep copy of a section of a byte array.
/// @param b Byte array that will be sliced.
/// @param startIndex Index of first byte to copy.
/// @param endIndex Index of last byte to copy + 1.
/// @return A deep copy of b from startIndex to endIndex.
function publicDeepCopyBytes(
bytes memory b,
uint256 startIndex,
uint256 endIndex)
/// @dev Pops the last byte off of a byte array by modifying its length.
/// @param b Byte array that will be modified.
/// @return The byte that was popped off.
function publicPopByte(bytes memory b)
public
pure
returns (bytes memory copy)
returns (bytes memory, byte result)
{
copy = deepCopyBytes(b, startIndex, endIndex);
return copy;
result = popByte(b);
return (b, result);
}
/// @dev Pops the last 20 bytes off of a byte array by modifying its length.
/// @param b Byte array that will be modified.
/// @return The 20 byte address that was popped off.
function publicPopAddress(bytes memory b)
public
returns (bytes memory, address result)
{
result = popAddress(b);
return (b, result);
}
/// @dev Tests equality of two byte arrays.

2
packages/contracts/src/contracts/current/test/Whitelist/Whitelist.sol
View File

@@ -75,7 +75,7 @@ contract Whitelist is
// This contract must be the entry point for the transaction.
require(takerAddress == tx.origin);
// Check if maker is on the whitelist
// Check if maker is on the whitelist.
require(
isWhitelisted[order.makerAddress],
ADDRESS_NOT_WHITELISTED

54
packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol
View File

@@ -27,39 +27,45 @@ contract LibBytes {
string constant GTE_32_LENGTH_REQUIRED = "Length must be greater than or equal to 32.";
string constant INDEX_OUT_OF_BOUNDS = "Specified array index is out of bounds.";
/// @dev Performs a deep copy of a section of a byte array.
/// @param b Byte array that will be copied.
/// @param index Index of first byte to copy.
/// @param len Number of bytes to copy starting from index.
/// @return A deep copy `len` bytes of b starting from index.
function deepCopyBytes(
bytes memory b,
uint256 index,
uint256 len)
/// @dev Pops the last byte off of a byte array by modifying its length.
/// @param b Byte array that will be modified.
/// @return The byte that was popped off.
function popByte(bytes memory b)
internal
pure
returns (bytes memory)
returns (byte result)
{
require(
b.length >= index + len,
INDEX_OUT_OF_BOUNDS
b.length > 0,
GT_ZERO_LENGTH_REQUIRED
);
bytes memory copy = new bytes(len);
result = b[b.length - 1];
assembly {
let newLen := sub(mload(b), 1)
mstore(b, newLen)
}
result;
}
// Arrays are prefixed by a 256 bit length parameter
index += 32;
/// @dev Pops the last 20 bytes off of a byte array by modifying its length.
/// @param b Byte array that will be modified.
/// @return The 20 byte address that was popped off.
function popAddress(bytes memory b)
internal
returns (address result)
{
require(
b.length >= 20,
GTE_20_LENGTH_REQUIRED
);
result = readAddress(b, b.length - 20);
assembly {
// Start storing onto copy after length field
let storeStartIndex := add(copy, 32)
// Start loading from b at index
let startLoadIndex := add(b, index)
for {let i := 0} lt(i, len) {i := add(i, 32)} {
mstore(add(storeStartIndex, i), mload(add(startLoadIndex, i)))
}
let newLen := sub(mload(b), 20)
mstore(b, newLen)
}
return copy;
return result;
}
/// @dev Tests equality of two byte arrays.

4
packages/contracts/src/utils/signing_utils.ts
View File

@@ -8,19 +8,19 @@ export const signingUtils = {
const prefixedMessage = ethUtil.hashPersonalMessage(message);
const ecSignature = ethUtil.ecsign(prefixedMessage, privateKey);
const signature = Buffer.concat([
ethUtil.toBuffer(signatureType),
ethUtil.toBuffer(ecSignature.v),
ecSignature.r,
ecSignature.s,
ethUtil.toBuffer(signatureType),
]);
return signature;
} else if (signatureType === SignatureType.EIP712) {
const ecSignature = ethUtil.ecsign(message, privateKey);
const signature = Buffer.concat([
ethUtil.toBuffer(signatureType),
ethUtil.toBuffer(ecSignature.v),
ecSignature.r,
ecSignature.s,
ethUtil.toBuffer(signatureType),
]);
return signature;
} else {

5
packages/contracts/test/exchange/core.ts
View File

@@ -460,10 +460,11 @@ describe('Exchange core', () => {
makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18),
});
const v = ethUtil.toBuffer(signedOrder.signature.slice(0, 4));
const invalidR = ethUtil.sha3('invalidR');
const invalidS = ethUtil.sha3('invalidS');
const signatureTypeAndV = signedOrder.signature.slice(0, 6);
const invalidSigBuff = Buffer.concat([ethUtil.toBuffer(signatureTypeAndV), invalidR, invalidS]);
const signatureType = ethUtil.toBuffer(`0x${signedOrder.signature.slice(-2)}`);
const invalidSigBuff = Buffer.concat([v, invalidR, invalidS, signatureType]);
const invalidSigHex = `0x${invalidSigBuff.toString('hex')}`;
signedOrder.signature = invalidSigHex;
return expect(exchangeWrapper.fillOrderAsync(signedOrder, takerAddress)).to.be.rejectedWith(

8
packages/contracts/test/exchange/signature_validator.ts
View File

@@ -75,13 +75,11 @@ describe('MixinSignatureValidator', () => {
});
it('should return false with an invalid signature', async () => {
const v = ethUtil.toBuffer(signedOrder.signature.slice(0, 4));
const invalidR = ethUtil.sha3('invalidR');
const invalidS = ethUtil.sha3('invalidS');
const invalidSigBuff = Buffer.concat([
ethUtil.toBuffer(signedOrder.signature.slice(0, 6)),
invalidR,
invalidS,
]);
const signatureType = ethUtil.toBuffer(`0x${signedOrder.signature.slice(-2)}`);
const invalidSigBuff = Buffer.concat([v, invalidR, invalidS, signatureType]);
const invalidSigHex = `0x${invalidSigBuff.toString('hex')}`;
signedOrder.signature = invalidSigHex;
const orderHashHex = orderUtils.getOrderHashHex(signedOrder);

53
packages/contracts/test/libraries/lib_bytes.ts
View File

@@ -22,6 +22,7 @@ describe('LibBytes', () => {
let owner: string;
let libBytes: TestLibBytesContract;
const byteArrayShorterThan32Bytes = '0x012345';
const byteArrayShorterThan20Bytes = byteArrayShorterThan32Bytes;
const byteArrayLongerThan32Bytes =
'0x0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef';
const byteArrayLongerThan32BytesFirstBytesSwapped =
@@ -60,39 +61,35 @@ describe('LibBytes', () => {
await blockchainLifecycle.revertAsync();
});
describe('deepCopyBytes', () => {
const byteArrayLongerThan32BytesLen = (byteArrayLongerThan32Bytes.length - 2) / 2;
it('should return a byte array of length 0 if len is 0', async () => {
const index = new BigNumber(0);
const len = new BigNumber(0);
const copy = await libBytes.publicDeepCopyBytes.callAsync(byteArrayLongerThan32Bytes, index, len);
expect(copy).to.equal(constants.NULL_BYTES);
describe('popByte', () => {
it('should revert if length is 0', async () => {
return expect(libBytes.publicPopByte.callAsync(constants.NULL_BYTES)).to.be.rejectedWith(constants.REVERT);
});
it('should throw if start index + length to copy is greater than length of byte array', async () => {
const index = new BigNumber(0);
const len = new BigNumber(byteArrayLongerThan32BytesLen + 1);
return expect(
libBytes.publicDeepCopyBytes.callAsync(byteArrayLongerThan32Bytes, index, len),
).to.be.rejectedWith(constants.REVERT);
});
it('should copy the entire byte array if index = 0 and len = b.length', async () => {
const index = new BigNumber(0);
const len = new BigNumber(byteArrayLongerThan32BytesLen);
const copy = await libBytes.publicDeepCopyBytes.callAsync(byteArrayLongerThan32Bytes, index, len);
expect(copy).to.equal(byteArrayLongerThan32Bytes);
});
it('should copy part of the byte array if area to copy is less than b.length', async () => {
const index = new BigNumber(10);
const len = new BigNumber(4);
const copy = await libBytes.publicDeepCopyBytes.callAsync(byteArrayLongerThan32Bytes, index, len);
const expectedCopy = `0x${byteArrayLongerThan32Bytes.slice(22, 30)}`;
expect(copy).to.equal(expectedCopy);
it('should pop the last byte from the input and return it', async () => {
const [newBytes, poppedByte] = await libBytes.publicPopByte.callAsync(byteArrayLongerThan32Bytes);
const expectedNewBytes = byteArrayLongerThan32Bytes.slice(0, -2);
const expectedPoppedByte = `0x${byteArrayLongerThan32Bytes.slice(-2)}`;
expect(newBytes).to.equal(expectedNewBytes);
expect(poppedByte).to.equal(expectedPoppedByte);
});
});
describe('popAddress', () => {
it('should revert if length is less than 20', async () => {
return expect(libBytes.publicPopAddress.callAsync(byteArrayShorterThan20Bytes)).to.be.rejectedWith(
constants.REVERT,
);
});
it('should pop the last 20 bytes from the input and return it', async () => {
const [newBytes, poppedAddress] = await libBytes.publicPopAddress.callAsync(byteArrayLongerThan32Bytes);
const expectedNewBytes = byteArrayLongerThan32Bytes.slice(0, -40);
const expectedPoppedAddress = `0x${byteArrayLongerThan32Bytes.slice(-40)}`;
expect(newBytes).to.equal(expectedNewBytes);
expect(poppedAddress).to.equal(expectedPoppedAddress);
});
});
describe('areBytesEqual', () => {
it('should return true if byte arrays are equal (both arrays < 32 bytes)', async () => {
const areBytesEqual = await libBytes.publicAreBytesEqual.callAsync(