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.

Signed-off-by: Adrian Sutton <adrian.sutton@consensys.net>
pull/2/head
Adrian Sutton 6 years ago committed by GitHub
parent 8380254edd
commit b18c9ccdc0
  1. 4
      ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/mainnet/precompiles/ECRECPrecompiledContract.java
  2. 12
      ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/vm/AbstractCallOperation.java
  3. 3
      ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/vm/Memory.java
  4. 24
      ethereum/core/src/test/java/tech/pegasys/pantheon/ethereum/vm/MemoryTest.java

@ -72,7 +72,7 @@ public class ECRECPrecompiledContract extends AbstractPrecompiledContract {
try {
final Optional<PublicKey> 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;
}
}
}

@ -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());

@ -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;

@ -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));
}

Loading…
Cancel
Save