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

Remove pragma experimental v0.5.0 and use staticcall is assembly

Browse Source
This commit is contained in:
Amir Bandeali
2018-08-24 00:19:06 -07:00
parent c5f8b9c2d2
commit 0a1ae2c311
6 changed files with 198 additions and 15 deletions
Download Patch File Download Diff File Expand all files Collapse all files

84
packages/contracts/src/2.0.0/protocol/Exchange/MixinSignatureValidator.sol vendored
View File

@@ -17,7 +17,6 @@
*/
pragma solidity 0.4.24;
pragma experimental "v0.5.0";
import "../../utils/LibBytes/LibBytes.sol";
import "./mixins/MSignatureValidator.sol";
@@ -192,7 +191,11 @@ contract MixinSignatureValidator is
// Signature verified by wallet contract.
// If used with an order, the maker of the order is the wallet contract.
} else if (signatureType == SignatureType.Wallet) {
isValid = IWallet(signerAddress).isValidSignature(hash, signature);
isValid = isValidWalletSignature(
hash,
signerAddress,
signature
);
return isValid;
// Signature verified by validator contract.
@@ -210,7 +213,8 @@ contract MixinSignatureValidator is
if (!allowedValidators[signerAddress][validatorAddress]) {
return false;
}
isValid = IValidator(validatorAddress).isValidSignature(
isValid = isValidValidatorSignature(
validatorAddress,
hash,
signerAddress,
signature
@@ -258,4 +262,78 @@ contract MixinSignatureValidator is
// signature was invalid.)
revert("SIGNATURE_UNSUPPORTED");
}
/// @dev Verifies signature using logic defined by Wallet contract.
/// @param hash Any 32 byte hash.
/// @param walletAddress Address that should have signed the given hash
/// and defines its own signature verification method.
/// @param signature Proof that the hash has been signed by signer.
/// @return True if signature is valid for given wallet..
function isValidWalletSignature(
bytes32 hash,
address walletAddress,
bytes signature
)
internal
view
returns (bool isValid)
{
bytes memory calldata = abi.encodeWithSelector(
IWallet(walletAddress).isValidSignature.selector,
hash,
signature
);
assembly {
let cdStart := add(calldata, 32)
let success := staticcall(
gas, // forward all gas
walletAddress, // address of Wallet contract
cdStart, // pointer to start of input
mload(calldata), // length of input
cdStart, // write input over output
32 // output size is 32 bytes
)
// Signature is valid if call did not revert and returned true
isValid := and(success, mload(cdStart))
}
return isValid;
}
/// @dev Verifies signature using logic defined by Validator contract.
/// @param validatorAddress Address of validator contract.
/// @param hash Any 32 byte hash.
/// @param signerAddress Address that should have signed the given hash.
/// @param signature Proof that the hash has been signed by signer.
/// @return True if the address recovered from the provided signature matches the input signer address.
function isValidValidatorSignature(
address validatorAddress,
bytes32 hash,
address signerAddress,
bytes signature
)
internal
view
returns (bool isValid)
{
bytes memory calldata = abi.encodeWithSelector(
IValidator(signerAddress).isValidSignature.selector,
hash,
signerAddress,
signature
);
assembly {
let cdStart := add(calldata, 32)
let success := staticcall(
gas, // forward all gas
validatorAddress, // address of Validator contract
cdStart, // pointer to start of input
mload(calldata), // length of input
cdStart, // write input over output
32 // output size is 32 bytes
)
// Signature is valid if call did not revert and returned true
isValid := and(success, mload(cdStart))
}
return isValid;
}
}

31
packages/contracts/src/2.0.0/protocol/Exchange/mixins/MSignatureValidator.sol vendored
View File

@@ -43,4 +43,35 @@ contract MSignatureValidator is
Trezor, // 0x08
NSignatureTypes // 0x09, number of signature types. Always leave at end.
}
/// @dev Verifies signature using logic defined by Wallet contract.
/// @param hash Any 32 byte hash.
/// @param walletAddress Address that should have signed the given hash
/// and defines its own signature verification method.
/// @param signature Proof that the hash has been signed by signer.
/// @return True if the address recovered from the provided signature matches the input signer address.
function isValidWalletSignature(
bytes32 hash,
address walletAddress,
bytes signature
)
internal
view
returns (bool isValid);
/// @dev Verifies signature using logic defined by Validator contract.
/// @param validatorAddress Address of validator contract.
/// @param hash Any 32 byte hash.
/// @param signerAddress Address that should have signed the given hash.
/// @param signature Proof that the hash has been signed by signer.
/// @return True if the address recovered from the provided signature matches the input signer address.
function isValidValidatorSignature(
address validatorAddress,
bytes32 hash,
address signerAddress,
bytes signature
)
internal
view
returns (bool isValid);
}

1
packages/contracts/src/2.0.0/test/TestSignatureValidator/TestSignatureValidator.sol vendored
View File

@@ -17,7 +17,6 @@
*/
pragma solidity 0.4.24;
pragma experimental "v0.5.0";
import "../../protocol/Exchange/MixinSignatureValidator.sol";
import "../../protocol/Exchange/MixinTransactions.sol";

16
packages/contracts/src/2.0.0/test/TestStaticCall/TestStaticCall.sol vendored
View File

@@ -18,6 +18,8 @@
pragma solidity 0.4.24;
import "../../tokens/ERC20Token/IERC20Token.sol";
contract TestStaticCall {
@@ -55,6 +57,20 @@ contract TestStaticCall {
return true;
}
/// @dev Approves an ERC20 token to spend tokens from this address.
/// @param token Address of ERC20 token.
/// @param spender Address that will spend tokens.
/// @param value Amount of tokens spender is approved to spend.
function approveERC20(
address token,
address spender,
uint256 value
)
external
{
IERC20Token(token).approve(spender, value);
}
/// @dev Increments state variable.
function updateState()
internal

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

@@ -1,6 +1,6 @@
import { BlockchainLifecycle } from '@0xproject/dev-utils';
import { assetDataUtils, orderHashUtils } from '@0xproject/order-utils';
import { RevertReason, SignedOrder } from '@0xproject/types';
import { RevertReason, SignatureType, SignedOrder } from '@0xproject/types';
import { BigNumber } from '@0xproject/utils';
import { Web3Wrapper } from '@0xproject/web3-wrapper';
import * as chai from 'chai';
@@ -14,6 +14,7 @@ import { DummyNoReturnERC20TokenContract } from '../../generated_contract_wrappe
import { ERC20ProxyContract } from '../../generated_contract_wrappers/erc20_proxy';
import { ERC721ProxyContract } from '../../generated_contract_wrappers/erc721_proxy';
import { ExchangeCancelEventArgs, ExchangeContract } from '../../generated_contract_wrappers/exchange';
import { TestStaticCallContract } from '../../generated_contract_wrappers/test_static_call';
import { artifacts } from '../utils/artifacts';
import { expectTransactionFailedAsync } from '../utils/assertions';
import { getLatestBlockTimestampAsync, increaseTimeAndMineBlockAsync } from '../utils/block_timestamp';
@@ -44,6 +45,8 @@ describe('Exchange core', () => {
let exchange: ExchangeContract;
let erc20Proxy: ERC20ProxyContract;
let erc721Proxy: ERC721ProxyContract;
let maliciousWallet: TestStaticCallContract;
let maliciousValidator: TestStaticCallContract;
let signedOrder: SignedOrder;
let erc20Balances: ERC20BalancesByOwner;
@@ -109,6 +112,12 @@ describe('Exchange core', () => {
constants.AWAIT_TRANSACTION_MINED_MS,
);
maliciousWallet = maliciousValidator = await TestStaticCallContract.deployFrom0xArtifactAsync(
artifacts.TestStaticCall,
provider,
txDefaults,
);
defaultMakerAssetAddress = erc20TokenA.address;
defaultTakerAssetAddress = erc20TokenB.address;
@@ -161,6 +170,54 @@ describe('Exchange core', () => {
RevertReason.OrderUnfillable,
);
});
it('should revert if `isValidSignature` tries to update state when SignatureType=Wallet', async () => {
const maliciousMakerAddress = maliciousWallet.address;
await web3Wrapper.awaitTransactionSuccessAsync(
await erc20TokenA.setBalance.sendTransactionAsync(
maliciousMakerAddress,
constants.INITIAL_ERC20_BALANCE,
),
constants.AWAIT_TRANSACTION_MINED_MS,
);
await web3Wrapper.awaitTransactionSuccessAsync(
await maliciousWallet.approveERC20.sendTransactionAsync(
erc20TokenA.address,
erc20Proxy.address,
constants.INITIAL_ERC20_ALLOWANCE,
),
constants.AWAIT_TRANSACTION_MINED_MS,
);
signedOrder = await orderFactory.newSignedOrderAsync({
makerAddress: maliciousMakerAddress,
makerFee: constants.ZERO_AMOUNT,
});
signedOrder.signature = `0x0${SignatureType.Wallet}`;
await expectTransactionFailedAsync(
exchangeWrapper.fillOrderAsync(signedOrder, takerAddress),
RevertReason.InvalidOrderSignature,
);
});
it('should revert if `isValidSignature` tries to update state when SignatureType=Validator', async () => {
const isApproved = true;
await web3Wrapper.awaitTransactionSuccessAsync(
await exchange.setSignatureValidatorApproval.sendTransactionAsync(
maliciousValidator.address,
isApproved,
{ from: makerAddress },
),
constants.AWAIT_TRANSACTION_MINED_MS,
);
signedOrder = await orderFactory.newSignedOrderAsync({
makerAddress: maliciousValidator.address,
});
signedOrder.signature = `${maliciousValidator.address}0${SignatureType.Validator}`;
await expectTransactionFailedAsync(
exchangeWrapper.fillOrderAsync(signedOrder, takerAddress),
RevertReason.InvalidOrderSignature,
);
});
});
describe('Testing exchange of ERC20 tokens with no return values', () => {

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

@@ -352,7 +352,7 @@ describe('MixinSignatureValidator', () => {
expect(isValidSignature).to.be.false();
});
it('should not allow `isValidSignature` to update state when SignatureType=Wallet', async () => {
it('should return false when `isValidSignature` attempts to update state and not allow state to be updated when SignatureType=Wallet', async () => {
// Create EIP712 signature
const orderHashHex = orderHashUtils.getOrderHashHex(signedOrder);
const orderHashBuffer = ethUtil.toBuffer(orderHashHex);
@@ -366,13 +366,12 @@ describe('MixinSignatureValidator', () => {
]);
const signatureHex = ethUtil.bufferToHex(signature);
// Validate signature
await expectContractCallFailedWithoutReasonAsync(
signatureValidator.publicIsValidSignature.callAsync(
orderHashHex,
maliciousWallet.address,
signatureHex,
),
const isValid = await signatureValidator.publicIsValidSignature.callAsync(
orderHashHex,
maliciousWallet.address,
signatureHex,
);
expect(isValid).to.be.equal(false);
});
it('should return true when SignatureType=Validator, signature is valid and validator is approved', async () => {
@@ -405,15 +404,18 @@ describe('MixinSignatureValidator', () => {
expect(isValidSignature).to.be.false();
});
it('should not allow `isValidSignature` to update state when SignatureType=Validator', async () => {
it('should return false when `isValidSignature` attempts to update state and not allow state to be updated when SignatureType=Validator', async () => {
const validatorAddress = ethUtil.toBuffer(`${maliciousValidator.address}`);
const signatureType = ethUtil.toBuffer(`0x${SignatureType.Validator}`);
const signature = Buffer.concat([validatorAddress, signatureType]);
const signatureHex = ethUtil.bufferToHex(signature);
const orderHashHex = orderHashUtils.getOrderHashHex(signedOrder);
await expectContractCallFailedWithoutReasonAsync(
signatureValidator.publicIsValidSignature.callAsync(orderHashHex, signerAddress, signatureHex),
const isValid = await signatureValidator.publicIsValidSignature.callAsync(
orderHashHex,
maliciousValidator.address,
signatureHex,
);
expect(isValid).to.be.equal(false);
});
it('should return false when SignatureType=Validator, signature is valid and validator is not approved', async () => {
// Set approval of signature validator to false