diff --git a/contracts/exchange/contracts/test/ReentrancyTester.sol b/contracts/exchange/contracts/test/ReentrancyTester.sol index 05c845e160..897dbecd1b 100644 --- a/contracts/exchange/contracts/test/ReentrancyTester.sol +++ b/contracts/exchange/contracts/test/ReentrancyTester.sol @@ -65,13 +65,12 @@ contract ReentrancyTester is } /// @dev Overridden to revert on unsuccessful fillOrder call. - function fillOrderNoThrow( + function _fillOrderNoThrow( LibOrder.Order memory order, uint256 takerAssetFillAmount, bytes memory signature ) - public - payable + internal returns (LibFillResults.FillResults memory fillResults) { // ABI encode calldata for `fillOrder` diff --git a/contracts/exchange/test/signature_validator.ts b/contracts/exchange/test/signature_validator.ts index 9ebd9ae248..7ba9dcde48 100644 --- a/contracts/exchange/test/signature_validator.ts +++ b/contracts/exchange/test/signature_validator.ts @@ -575,6 +575,27 @@ blockchainTests.resets('MixinSignatureValidator', env => { return expect(tx).to.revertWith(expectedError); }); + it('should revert when SignatureType=Validator and signature is shorter than 21 bytes', async () => { + // Set approval of signature validator to false + await signatureValidator.setSignatureValidatorApproval.awaitTransactionSuccessAsync( + validatorWallet.address, + false, + { from: signedOrder.makerAddress }, + ); + // Doesn't have to contain a real signature since our wallet contract + // just does a hash comparison. + const signatureHex = hexConcat(SignatureType.Validator); + const orderHashHex = orderHashUtils.getOrderHashHex(signedOrder); + const expectedError = new ExchangeRevertErrors.SignatureError( + ExchangeRevertErrors.SignatureErrorCode.InvalidLength, + orderHashHex, + signedOrder.makerAddress, + signatureHex, + ); + const tx = validateAsync(signedOrder, signatureHex, ValidatorWalletAction.MatchSignatureHash); + return expect(tx).to.revertWith(expectedError); + }); + it('should revert when SignatureType=Validator, signature is valid and validator is not approved', async () => { // Set approval of signature validator to false await signatureValidator.setSignatureValidatorApproval.awaitTransactionSuccessAsync( @@ -837,6 +858,27 @@ blockchainTests.resets('MixinSignatureValidator', env => { expect(isValidSignature).to.be.false(); }); + it('should revert when SignatureType=Validator and signature is shorter than 21 bytes', async () => { + // Set approval of signature validator to false + await signatureValidator.setSignatureValidatorApproval.awaitTransactionSuccessAsync( + validatorWallet.address, + false, + { from: signedTransaction.signerAddress }, + ); + // Doesn't have to contain a real signature since our wallet contract + // just does a hash comparison. + const signatureHex = hexConcat(SignatureType.Validator); + const transactionHashHex = transactionHashUtils.getTransactionHashHex(signedTransaction); + const expectedError = new ExchangeRevertErrors.SignatureError( + ExchangeRevertErrors.SignatureErrorCode.InvalidLength, + transactionHashHex, + signedTransaction.signerAddress, + signatureHex, + ); + const tx = validateAsync(signedTransaction, signatureHex, ValidatorWalletAction.MatchSignatureHash); + return expect(tx).to.revertWith(expectedError); + }); + it('should revert when validator returns nothing and SignatureType=Validator', async () => { const signatureDataHex = generateRandomSignature(); const signatureHex = hexConcat(signatureDataHex, validatorWallet.address, SignatureType.Validator); diff --git a/contracts/utils/test/lib_bytes.ts b/contracts/utils/test/lib_bytes.ts index 0d79c05852..ace87b530e 100644 --- a/contracts/utils/test/lib_bytes.ts +++ b/contracts/utils/test/lib_bytes.ts @@ -668,7 +668,7 @@ describe('LibBytes', () => { describe('writeBytesWithLength', () => { it('should successfully write short, nested array of bytes when it takes up the whole array', async () => { const testBytesOffset = new BigNumber(0); - const emptyByteArray = ethUtil.bufferToHex(ethUtil.toBuffer(shortTestBytesAsBuffer.byteLength)); + const emptyByteArray = ethUtil.bufferToHex(Buffer.alloc(shortTestBytesAsBuffer.byteLength)); const bytesWritten = await libBytes.publicWriteBytesWithLength.callAsync( emptyByteArray, testBytesOffset, @@ -683,7 +683,7 @@ describe('LibBytes', () => { const prefixDataAsBuffer = ethUtil.toBuffer(prefixData); const prefixOffset = new BigNumber(0); const emptyByteArray = ethUtil.bufferToHex( - ethUtil.toBuffer(prefixDataAsBuffer.byteLength + shortTestBytesAsBuffer.byteLength), + Buffer.alloc(prefixDataAsBuffer.byteLength + shortTestBytesAsBuffer.byteLength), ); let bytesWritten = await libBytes.publicWriteBytesWithLength.callAsync( emptyByteArray, @@ -703,7 +703,7 @@ describe('LibBytes', () => { }); it('should successfully write a nested array of bytes - one word in length - when it takes up the whole array', async () => { const testBytesOffset = new BigNumber(0); - const emptyByteArray = ethUtil.bufferToHex(ethUtil.toBuffer(wordOfTestBytesAsBuffer.byteLength)); + const emptyByteArray = ethUtil.bufferToHex(Buffer.alloc(wordOfTestBytesAsBuffer.byteLength)); const bytesWritten = await libBytes.publicWriteBytesWithLength.callAsync( emptyByteArray, testBytesOffset, @@ -718,7 +718,7 @@ describe('LibBytes', () => { const prefixDataAsBuffer = ethUtil.toBuffer(prefixData); const prefixOffset = new BigNumber(0); const emptyByteArray = ethUtil.bufferToHex( - ethUtil.toBuffer(prefixDataAsBuffer.byteLength + wordOfTestBytesAsBuffer.byteLength), + Buffer.alloc(prefixDataAsBuffer.byteLength + wordOfTestBytesAsBuffer.byteLength), ); let bytesWritten = await libBytes.publicWriteBytesWithLength.callAsync( emptyByteArray, @@ -738,7 +738,7 @@ describe('LibBytes', () => { }); it('should successfully write a long, nested bytes when it takes up the whole array', async () => { const testBytesOffset = new BigNumber(0); - const emptyByteArray = ethUtil.bufferToHex(ethUtil.toBuffer(longTestBytesAsBuffer.byteLength)); + const emptyByteArray = ethUtil.bufferToHex(Buffer.alloc(longTestBytesAsBuffer.byteLength)); const bytesWritten = await libBytes.publicWriteBytesWithLength.callAsync( emptyByteArray, testBytesOffset, @@ -753,7 +753,7 @@ describe('LibBytes', () => { const prefixDataAsBuffer = ethUtil.toBuffer(prefixData); const prefixOffset = new BigNumber(0); const emptyByteArray = ethUtil.bufferToHex( - ethUtil.toBuffer(prefixDataAsBuffer.byteLength + longTestBytesAsBuffer.byteLength), + Buffer.alloc(prefixDataAsBuffer.byteLength + longTestBytesAsBuffer.byteLength), ); let bytesWritten = await libBytes.publicWriteBytesWithLength.callAsync( emptyByteArray, @@ -769,7 +769,7 @@ describe('LibBytes', () => { }); it('should fail if the byte array is too short to hold the length of a nested byte array', async () => { const offset = new BigNumber(0); - const emptyByteArray = ethUtil.bufferToHex(ethUtil.toBuffer(1)); + const emptyByteArray = ethUtil.bufferToHex(Buffer.alloc(1)); const inputLen = new BigNumber((longData.length - 2) / 2); const expectedError = new LibBytesRevertErrors.InvalidByteOperationError( LibBytesRevertErrors.InvalidByteOperationErrorCodes.LengthGreaterThanOrEqualsNestedBytesLengthRequired, @@ -781,7 +781,7 @@ describe('LibBytes', () => { ).to.revertWith(expectedError); }); it('should fail if the length between the offset and end of the byte array is too short to hold the length of a nested byte array', async () => { - const emptyByteArray = ethUtil.bufferToHex(ethUtil.toBuffer(shortTestBytesAsBuffer.byteLength)); + const emptyByteArray = ethUtil.bufferToHex(Buffer.alloc(shortTestBytesAsBuffer.byteLength)); const badOffset = new BigNumber(ethUtil.toBuffer(shortTestBytesAsBuffer).byteLength); const inputLen = new BigNumber((shortData.length - 2) / 2); const expectedError = new LibBytesRevertErrors.InvalidByteOperationError(