From 41bce807f5734174dc032435131494abbe5b2749 Mon Sep 17 00:00:00 2001 From: Adrian Sutton Date: Sun, 4 Nov 2018 10:14:50 +0100 Subject: [PATCH] Treat output length as a maximum length for CALL operations (#236) * Fix corner case where Memory did not clear target destination if the new bytes to set was empty. * Use BytesValue.EMPTY instead of Bytes32.EMPTY to make it clear that it's a 0 length BytesValues rather than 32 bytes of zeross. * Treat outputLength for call operations as a maximum length - when the actual data to output is shorter than outputLength, do not zero out the tail end of the memory range. --- .../precompiles/ECRECPrecompiledContract.java | 4 ++-- .../ethereum/vm/AbstractCallOperation.java | 12 ++++++++-- .../pegasys/pantheon/ethereum/vm/Memory.java | 3 ++- .../pantheon/ethereum/vm/MemoryTest.java | 24 +++++++++++++++++++ 4 files changed, 38 insertions(+), 5 deletions(-) diff --git a/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/mainnet/precompiles/ECRECPrecompiledContract.java b/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/mainnet/precompiles/ECRECPrecompiledContract.java index 0f994139fd..49ec07432e 100644 --- a/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/mainnet/precompiles/ECRECPrecompiledContract.java +++ b/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/mainnet/precompiles/ECRECPrecompiledContract.java @@ -72,7 +72,7 @@ public class ECRECPrecompiledContract extends AbstractPrecompiledContract { try { final Optional recovered = PublicKey.recoverFromSignature(h, signature); if (!recovered.isPresent()) { - return Bytes32.EMPTY; + return BytesValue.EMPTY; } final Bytes32 hashed = Hash.hash(recovered.get().getEncodedBytes()); @@ -80,7 +80,7 @@ public class ECRECPrecompiledContract extends AbstractPrecompiledContract { hashed.slice(12).copyTo(result, 12); return result; } catch (final IllegalArgumentException e) { - return Bytes32.EMPTY; + return BytesValue.EMPTY; } } } diff --git a/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/vm/AbstractCallOperation.java b/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/vm/AbstractCallOperation.java index 7ce21db429..6a1fcb4fe6 100644 --- a/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/vm/AbstractCallOperation.java +++ b/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/vm/AbstractCallOperation.java @@ -195,9 +195,17 @@ public abstract class AbstractCallOperation extends AbstractOperation { final UInt256 outputOffset = outputDataOffset(frame); final UInt256 outputSize = outputDataLength(frame); - frame.writeMemory(outputOffset, outputSize, childFrame.getOutputData()); + final BytesValue outputData = childFrame.getOutputData(); + final int outputSizeAsInt = outputSize.toInt(); - frame.setReturnData(childFrame.getOutputData()); + if (outputSizeAsInt > outputData.size()) { + frame.expandMemory(outputOffset.toLong(), outputSizeAsInt); + frame.writeMemory(outputOffset, UInt256.of(outputData.size()), outputData); + } else { + frame.writeMemory(outputOffset, outputSize, outputData); + } + + frame.setReturnData(outputData); frame.addLogs(childFrame.getLogs()); frame.addSelfDestructs(childFrame.getSelfDestructs()); frame.incrementGasRefund(childFrame.getGasRefund()); diff --git a/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/vm/Memory.java b/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/vm/Memory.java index 84d70dee20..f9cb088db4 100644 --- a/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/vm/Memory.java +++ b/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/vm/Memory.java @@ -361,8 +361,9 @@ public class Memory { ensureCapacityForBytes(start, length); // We've properly expanded memory as needed. We now have simply have to copy the - // min(length, value.size()) first bytes of value. + // min(length, value.size()) first bytes of value and clear any bytes that exceed value's length if (taintedValue.isEmpty()) { + clearBytes(location, numBytes); return; } final BytesValue value; diff --git a/ethereum/core/src/test/java/tech/pegasys/pantheon/ethereum/vm/MemoryTest.java b/ethereum/core/src/test/java/tech/pegasys/pantheon/ethereum/vm/MemoryTest.java index 1f6c63512f..0ccc6b9e8f 100644 --- a/ethereum/core/src/test/java/tech/pegasys/pantheon/ethereum/vm/MemoryTest.java +++ b/ethereum/core/src/test/java/tech/pegasys/pantheon/ethereum/vm/MemoryTest.java @@ -123,6 +123,30 @@ public class MemoryTest { assertThat(memory.getWord(UInt256.of(64))).isEqualTo(WORD3); } + @Test + public void shouldClearMemoryWhenSourceDataIsEmpty() { + memory.setWord(UInt256.of(64), WORD3); + assertThat(memory.getWord(UInt256.of(64))).isEqualTo(WORD3); + + memory.setBytes(UInt256.ZERO, UInt256.of(96), BytesValue.EMPTY); + + assertThat(memory.getWord(UInt256.of(0))).isEqualTo(Bytes32.ZERO); + assertThat(memory.getWord(UInt256.of(32))).isEqualTo(Bytes32.ZERO); + assertThat(memory.getWord(UInt256.of(64))).isEqualTo(Bytes32.ZERO); + } + + @Test + public void shouldClearMemoryWhenSourceDataIsEmptyWithSourceOffset() { + memory.setWord(UInt256.of(64), WORD3); + assertThat(memory.getWord(UInt256.of(64))).isEqualTo(WORD3); + + memory.setBytes(UInt256.ZERO, UInt256.ZERO, UInt256.of(96), BytesValue.EMPTY); + + assertThat(memory.getWord(UInt256.of(0))).isEqualTo(Bytes32.ZERO); + assertThat(memory.getWord(UInt256.of(32))).isEqualTo(Bytes32.ZERO); + assertThat(memory.getWord(UInt256.of(64))).isEqualTo(Bytes32.ZERO); + } + private static Bytes32 fillBytes32(final long value) { return Bytes32.fromHexString(Strings.repeat(Long.toString(value), 64)); }