diff --git a/contracts/asset-proxy/contracts/src/ERC1155Proxy.sol b/contracts/asset-proxy/contracts/src/ERC1155Proxy.sol index c71484d584..bc1b1cdc7a 100644 --- a/contracts/asset-proxy/contracts/src/ERC1155Proxy.sol +++ b/contracts/asset-proxy/contracts/src/ERC1155Proxy.sol @@ -178,6 +178,14 @@ contract ERC1155Proxy is // End of asset data in calldata // +32 for length field let assetDataEnd := add(assetDataOffset, add(assetDataLength, 32)) + if gt(assetDataEnd, calldatasize()) { + // Revert with `Error("INVALID_ASSET_DATA")` + mstore(0, 0x08c379a000000000000000000000000000000000000000000000000000000000) + mstore(32, 0x0000002000000000000000000000000000000000000000000000000000000000) + mstore(64, 0x00000012494e56414c49445f41535345545f4441544100000000000000000000) + mstore(96, 0) + revert(0, 100) + } // Load offset to parameters section in asset data let paramsInAssetDataOffset := add(assetDataOffset, 36) diff --git a/contracts/asset-proxy/test/erc1155_proxy.ts b/contracts/asset-proxy/test/erc1155_proxy.ts index 2bee69c819..33e6df7956 100644 --- a/contracts/asset-proxy/test/erc1155_proxy.ts +++ b/contracts/asset-proxy/test/erc1155_proxy.ts @@ -402,7 +402,7 @@ describe('ERC1155Proxy', () => { const expectedInitialBalances = [spenderInitialFungibleBalance, receiverContractInitialFungibleBalance]; await erc1155Wrapper.assertBalancesAsync(tokenHolders, tokensToTransfer, expectedInitialBalances); // execute transfer - const txReceipt = await erc1155ProxyWrapper.transferFromWithLogsAsync( + const txReceipt = await erc1155ProxyWrapper.transferFromAsync( spender, receiverContract, erc1155Contract.address, @@ -446,7 +446,7 @@ describe('ERC1155Proxy', () => { await erc1155Wrapper.assertBalancesAsync(tokenHolders, tokensToTransfer, expectedInitialBalances); // execute transfer const nullReceiverCallbackData = '0x'; - const txReceipt = await erc1155ProxyWrapper.transferFromWithLogsAsync( + const txReceipt = await erc1155ProxyWrapper.transferFromAsync( spender, receiverContract, erc1155Contract.address, @@ -494,7 +494,7 @@ describe('ERC1155Proxy', () => { const oneWordInBytes = 32; expect(customReceiverCallbackDataAsBuffer.byteLength).to.be.equal(oneWordInBytes); // execute transfer - const txReceipt = await erc1155ProxyWrapper.transferFromWithLogsAsync( + const txReceipt = await erc1155ProxyWrapper.transferFromAsync( spender, receiverContract, erc1155Contract.address, @@ -545,7 +545,7 @@ describe('ERC1155Proxy', () => { const oneWordInBytes = 32; expect(customReceiverCallbackDataAsBuffer.byteLength).to.be.equal(oneWordInBytes * scalar); // execute transfer - const txReceipt = await erc1155ProxyWrapper.transferFromWithLogsAsync( + const txReceipt = await erc1155ProxyWrapper.transferFromAsync( spender, receiverContract, erc1155Contract.address, @@ -597,7 +597,7 @@ describe('ERC1155Proxy', () => { expect(customReceiverCallbackDataAsBuffer.byteLength).to.be.greaterThan(oneWordInBytes * scalar); expect(customReceiverCallbackDataAsBuffer.byteLength).to.be.lessThan(oneWordInBytes * (scalar + 1)); // execute transfer - const txReceipt = await erc1155ProxyWrapper.transferFromWithLogsAsync( + const txReceipt = await erc1155ProxyWrapper.transferFromAsync( spender, receiverContract, erc1155Contract.address, @@ -649,7 +649,7 @@ describe('ERC1155Proxy', () => { const expectedInitialBalances = [spenderInitialFungibleBalance, receiverContractInitialFungibleBalance]; await erc1155Wrapper.assertBalancesAsync(tokenHolders, tokensToTransfer, expectedInitialBalances); // execute transfer - const txReceipt = await erc1155ProxyWrapper.transferFromWithLogsAsync( + const txReceipt = await erc1155ProxyWrapper.transferFromAsync( spender, receiverContract, erc1155Contract.address, @@ -880,7 +880,7 @@ describe('ERC1155Proxy', () => { const assetData = `${assetDataSelectorAndContractAddress}${assetDataParameters}`; ///// Step 4/5 ///// // Transfer tokens - const txReceipt = await erc1155ProxyWrapper.transferFromWithLogsAsync( + const txReceipt = await erc1155ProxyWrapper.transferFromAsync( spender, receiverContract, erc1155Contract.address, @@ -1009,7 +1009,7 @@ describe('ERC1155Proxy', () => { const assetData = `${assetDataSelectorAndContractAddress}${assetDataParameters}`; ///// Step 4/5 ///// // Transfer tokens - const txReceipt = await erc1155ProxyWrapper.transferFromWithLogsAsync( + const txReceipt = await erc1155ProxyWrapper.transferFromAsync( spender, receiverContract, erc1155Contract.address, @@ -1352,6 +1352,80 @@ describe('ERC1155Proxy', () => { RevertReason.InvalidDataOffset, ); }); + it('should revert if asset data lies outside the bounds of calldata', async () => { + // setup test parameters + const tokensToTransfer = fungibleTokens.slice(0, 1); + const valuesToTransfer = [fungibleValueToTransferLarge]; + const valueMultiplier = valueMultiplierSmall; + const erc1155ContractAddress = erc1155Wrapper.getContract().address; + const assetData = assetDataUtils.encodeERC1155AssetData( + erc1155ContractAddress, + tokensToTransfer, + valuesToTransfer, + receiverCallbackData, + ); + const txData = erc1155ProxyWrapper.getTransferFromAbiEncodedTxData( + spender, + receiverContract, + erc1155Contract.address, + tokensToTransfer, + valuesToTransfer, + valueMultiplier, + receiverCallbackData, + authorized, + assetData, + ); + const offsetToAssetData = "0000000000000000000000000000000000000000000000000000000000000080"; + const invalidOffsetToAssetData = "0000000000000000000000000000000000000000000000000000000000000180"; + const badTxData = txData.replace(offsetToAssetData, invalidOffsetToAssetData); + + console.log(JSON.stringify(RevertReason.InvalidAssetData)); + // execute transfer + await expectTransactionFailedAsync( + erc1155ProxyWrapper.transferFromRawAsync( + badTxData, + authorized, + ), + RevertReason.InvalidAssetDataLength, + ); + }); + it('should revert if asset data lies outside the bounds of calldata', async () => { + // setup test parameters + const tokensToTransfer = fungibleTokens.slice(0, 1); + const valuesToTransfer = [fungibleValueToTransferLarge]; + const valueMultiplier = valueMultiplierSmall; + const erc1155ContractAddress = erc1155Wrapper.getContract().address; + const assetData = assetDataUtils.encodeERC1155AssetData( + erc1155ContractAddress, + tokensToTransfer, + valuesToTransfer, + receiverCallbackData, + ); + const txData = erc1155ProxyWrapper.getTransferFromAbiEncodedTxData( + spender, + receiverContract, + erc1155Contract.address, + tokensToTransfer, + valuesToTransfer, + valueMultiplier, + receiverCallbackData, + authorized, + assetData, + ); + // append asset data to end of tx data with a length of 0x300 bytes, which will extend past actual calldata. + const offsetToAssetData = "0000000000000000000000000000000000000000000000000000000000000080"; + const invalidOffsetToAssetData = "0000000000000000000000000000000000000000000000000000000000000200"; + const newAssetData = "0000000000000000000000000000000000000000000000000000000000000304"; + const badTxData = `${txData.replace(offsetToAssetData, invalidOffsetToAssetData)}${newAssetData}`; + // execute transfer + await expectTransactionFailedAsync( + erc1155ProxyWrapper.transferFromRawAsync( + badTxData, + authorized, + ), + RevertReason.InvalidAssetData, + ); + }); it('should revert if length of assetData, excluding the selector, is not a multiple of 32', async () => { // setup test parameters const tokensToTransfer = fungibleTokens.slice(0, 1); @@ -1368,7 +1442,7 @@ describe('ERC1155Proxy', () => { const assetDataWithExtraData = `${assetData}${extraData}`; // execute transfer await expectTransactionFailedAsync( - erc1155ProxyWrapper.transferFromWithLogsAsync( + erc1155ProxyWrapper.transferFromAsync( spender, receiverContract, erc1155Contract.address, @@ -1396,7 +1470,7 @@ describe('ERC1155Proxy', () => { const assetData131Bytes = `${AssetProxyId.ERC1155}${zeros96Bytes}`; // execute transfer await expectTransactionFailedAsync( - erc1155ProxyWrapper.transferFromWithLogsAsync( + erc1155ProxyWrapper.transferFromAsync( spender, receiverContract, erc1155Contract.address, diff --git a/contracts/asset-proxy/test/utils/erc1155_proxy_wrapper.ts b/contracts/asset-proxy/test/utils/erc1155_proxy_wrapper.ts index a84d2368db..fdac20b46a 100644 --- a/contracts/asset-proxy/test/utils/erc1155_proxy_wrapper.ts +++ b/contracts/asset-proxy/test/utils/erc1155_proxy_wrapper.ts @@ -84,6 +84,66 @@ export class ERC1155ProxyWrapper { this._validateProxyContractExistsOrThrow(); return this._proxyIdIfExists as string; } + /** + * @dev generates abi-encoded tx data for transferring erc1155 fungible/non-fungible tokens. + * @param from source address + * @param to destination address + * @param contractAddress address of erc155 contract + * @param tokensToTransfer array of erc1155 tokens to transfer + * @param valuesToTransfer array of corresponding values for each erc1155 token to transfer + * @param valueMultiplier each value in `valuesToTransfer` is multiplied by this + * @param receiverCallbackData callback data if `to` is a contract + * @param authorizedSender sender of `transferFrom` transaction + * @param extraData extra data to append to `transferFrom` transaction. Optional. + * @return abi encoded tx data. + */ + public getTransferFromAbiEncodedTxData( + from: string, + to: string, + contractAddress: string, + tokensToTransfer: BigNumber[], + valuesToTransfer: BigNumber[], + valueMultiplier: BigNumber, + receiverCallbackData: string, + authorizedSender: string, + assetData_?: string, + ): string { + this._validateProxyContractExistsOrThrow(); + const assetData = + assetData_ === undefined + ? assetDataUtils.encodeERC1155AssetData( + contractAddress, + tokensToTransfer, + valuesToTransfer, + receiverCallbackData, + ) + : assetData_; + const data = this._assetProxyInterface.transferFrom.getABIEncodedTransactionData( + assetData, + from, + to, + valueMultiplier, + ); + return data; + } + /** + * @dev transfers erc1155 fungible/non-fungible tokens. + * @param txData: abi-encoded tx data + * @param authorizedSender sender of `transferFrom` transaction + */ + public async transferFromRawAsync( + txData: string, + authorizedSender: string, + ): Promise { + const txHash = await this._web3Wrapper.sendTransactionAsync({ + to: (this._proxyContract as ERC1155ProxyContract).address, + data: txData, + from: authorizedSender, + gas: 300000, + }); + const txReceipt = await this._logDecoder.getTxWithDecodedLogsAsync(txHash); + return txReceipt; + } /** * @dev transfers erc1155 fungible/non-fungible tokens. * @param from source address @@ -107,7 +167,7 @@ export class ERC1155ProxyWrapper { receiverCallbackData: string, authorizedSender: string, assetData_?: string, - ): Promise { + ): Promise { this._validateProxyContractExistsOrThrow(); const assetData = assetData_ === undefined @@ -130,45 +190,7 @@ export class ERC1155ProxyWrapper { from: authorizedSender, gas: 300000, }); - return txHash; - } - /** - * @dev transfers erc1155 fungible/non-fungible tokens. - * @param from source address - * @param to destination address - * @param contractAddress address of erc155 contract - * @param tokensToTransfer array of erc1155 tokens to transfer - * @param valuesToTransfer array of corresponding values for each erc1155 token to transfer - * @param valueMultiplier each value in `valuesToTransfer` is multiplied by this - * @param receiverCallbackData callback data if `to` is a contract - * @param authorizedSender sender of `transferFrom` transaction - * @param extraData extra data to append to `transferFrom` transaction. Optional. - * @return tranasction receipt with decoded logs. - */ - public async transferFromWithLogsAsync( - from: string, - to: string, - contractAddress: string, - tokensToTransfer: BigNumber[], - valuesToTransfer: BigNumber[], - valueMultiplier: BigNumber, - receiverCallbackData: string, - authorizedSender: string, - assetData?: string, - ): Promise { - const txReceipt = await this._logDecoder.getTxWithDecodedLogsAsync( - await this.transferFromAsync( - from, - to, - contractAddress, - tokensToTransfer, - valuesToTransfer, - valueMultiplier, - receiverCallbackData, - authorizedSender, - assetData, - ), - ); + const txReceipt = await this._logDecoder.getTxWithDecodedLogsAsync(txHash); return txReceipt; } /**