Fix memory expansion bounds checking (#1322)

When a CALL series operation has an erroneous input offset (such as
starting at a ETH address instead of a real offset) we threw an
ArithmeticException.

* Restore the old memory bounds checking on memory expansion
* Treat these formerly uncaught exceptions as invalid transactions and
  report errors with a stack trace and custom halt reason.

Signed-off-by: Danno Ferrin <danno.ferrin@gmail.com>

ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/MainnetTransactionProcessor.java

@@ -27,6 +27,7 @@ import org.hyperledger.besu.ethereum.core.Wei;
 import org.hyperledger.besu.ethereum.core.WorldUpdater;
 import org.hyperledger.besu.ethereum.core.fees.CoinbaseFeePriceCalculator;
 import org.hyperledger.besu.ethereum.core.fees.TransactionPriceCalculator;
+import org.hyperledger.besu.ethereum.mainnet.TransactionValidator.TransactionInvalidReason;
 import org.hyperledger.besu.ethereum.vm.BlockHashLookup;
 import org.hyperledger.besu.ethereum.vm.Code;
 import org.hyperledger.besu.ethereum.vm.GasCalculator;
@@ -208,6 +209,7 @@ public class MainnetTransactionProcessor implements TransactionProcessor {
       final BlockHashLookup blockHashLookup,
       final Boolean isPersistingPrivateState,
       final TransactionValidationParams transactionValidationParams) {
+    try {
       LOG.trace("Starting execution of {}", transaction);
 
       ValidationResult<TransactionValidator.TransactionInvalidReason> validationResult =
@@ -234,7 +236,10 @@ public class MainnetTransactionProcessor implements TransactionProcessor {
       final Wei transactionGasPrice =
           transactionPriceCalculator.price(transaction, blockHeader.getBaseFee());
       LOG.trace(
-        "Incremented sender {} nonce ({} -> {})", senderAddress, previousNonce, sender.getNonce());
+          "Incremented sender {} nonce ({} -> {})",
+          senderAddress,
+          previousNonce,
+          sender.getNonce());
 
       final Wei upfrontGasCost = transaction.getUpfrontGasCost(transactionGasPrice);
       final Wei previousBalance = senderMutableAccount.decrementBalance(upfrontGasCost);
@@ -396,6 +401,13 @@ public class MainnetTransactionProcessor implements TransactionProcessor {
             validationResult,
             initialFrame.getRevertReason());
       }
+    } catch (final RuntimeException re) {
+      LOG.error("Critical Exception Processing Transaction", re);
+      return Result.invalid(
+          ValidationResult.invalid(
+              TransactionInvalidReason.INTERNAL_ERROR,
+              "Internal Error in Besu - " + re.toString()));
+    }
   }
 
   private static void clearEmptyAccounts(final WorldUpdater worldState) {

ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/TransactionValidator.java

@@ -87,6 +87,7 @@ public interface TransactionValidator {
     GAS_PRICE_TOO_LOW,
     TX_FEECAP_EXCEEDED,
     PRIVATE_VALUE_NOT_ZERO,
-    PRIVATE_UNIMPLEMENTED_TRANSACTION_TYPE;
+    PRIVATE_UNIMPLEMENTED_TRANSACTION_TYPE,
+    INTERNAL_ERROR;
   }
 }

ethereum/core/src/main/java/org/hyperledger/besu/ethereum/privacy/PrivateTransactionProcessor.java

@@ -29,6 +29,7 @@ import org.hyperledger.besu.ethereum.core.WorldUpdater;
 import org.hyperledger.besu.ethereum.mainnet.AbstractMessageProcessor;
 import org.hyperledger.besu.ethereum.mainnet.TransactionProcessor;
 import org.hyperledger.besu.ethereum.mainnet.TransactionValidator;
+import org.hyperledger.besu.ethereum.mainnet.TransactionValidator.TransactionInvalidReason;
 import org.hyperledger.besu.ethereum.mainnet.ValidationResult;
 import org.hyperledger.besu.ethereum.vm.BlockHashLookup;
 import org.hyperledger.besu.ethereum.vm.Code;
@@ -213,6 +214,7 @@ public class PrivateTransactionProcessor {
       final OperationTracer operationTracer,
       final BlockHashLookup blockHashLookup,
       final Bytes privacyGroupId) {
+    try {
       LOG.trace("Starting private execution of {}", transaction);
 
       final Address senderAddress = transaction.getSender();
@@ -335,6 +337,13 @@ public class PrivateTransactionProcessor {
                 TransactionValidator.TransactionInvalidReason.PRIVATE_TRANSACTION_FAILED),
             initialFrame.getRevertReason());
       }
+    } catch (final RuntimeException re) {
+      LOG.error("Critical Exception Processing Transaction", re);
+      return Result.invalid(
+          ValidationResult.invalid(
+              TransactionInvalidReason.INTERNAL_ERROR,
+              "Internal Error in Besu - " + re.toString()));
+    }
   }
 
   @SuppressWarnings("unused")

ethereum/core/src/main/java/org/hyperledger/besu/ethereum/vm/AbstractCallOperation.java

@@ -171,8 +171,8 @@ public abstract class AbstractCallOperation extends AbstractOperation {
       final Account account = frame.getWorldState().get(frame.getRecipientAddress());
       final Wei balance = account.getBalance();
       if (value(frame).compareTo(balance) > 0 || frame.getMessageStackDepth() >= 1024) {
-        frame.expandMemory(inputDataOffset(frame).intValue(), inputDataLength(frame).intValue());
+        frame.expandMemory(inputDataOffset(frame), inputDataLength(frame));
-        frame.expandMemory(outputDataOffset(frame).intValue(), outputDataLength(frame).intValue());
+        frame.expandMemory(outputDataOffset(frame), outputDataLength(frame));
         frame.incrementRemainingGas(gasAvailableForChildCall(frame).plus(cost));
         frame.popStackItems(getStackItemsConsumed());
         frame.pushStackItem(Bytes32.ZERO);
@@ -227,7 +227,7 @@ public abstract class AbstractCallOperation extends AbstractOperation {
     final int outputSizeAsInt = outputSize.intValue();
 
     if (outputSizeAsInt > outputData.size()) {
-      frame.expandMemory(outputOffset.intValue(), outputSizeAsInt);
+      frame.expandMemory(outputOffset, outputSize);
       frame.writeMemory(outputOffset, UInt256.valueOf(outputData.size()), outputData, true);
     } else {
       frame.writeMemory(outputOffset, outputSize, outputData, true);

ethereum/core/src/main/java/org/hyperledger/besu/ethereum/vm/EVM.java

@@ -84,7 +84,7 @@ public class EVM {
           logState(frame, result.getGasCost().orElse(Gas.ZERO));
           final Optional<ExceptionalHaltReason> haltReason = result.getHaltReason();
           if (haltReason.isPresent()) {
-            LOG.trace("MessageFrame evaluation halted because of {}", haltReason);
+            LOG.trace("MessageFrame evaluation halted because of {}", haltReason.get());
             frame.setExceptionalHaltReason(haltReason);
             frame.setState(State.EXCEPTIONAL_HALT);
           } else if (result.getGasCost().isPresent()) {

ethereum/core/src/main/java/org/hyperledger/besu/ethereum/vm/Memory.java

@@ -135,16 +135,28 @@ public class Memory {
   /**
    * Expands the active words to accommodate the specified byte position.
    *
-   * @param address The location in memory to start with.
+   * @param offset The location in memory to start with.
    * @param numBytes The number of bytes to get.
    */
-  void ensureCapacityForBytes(final int address, final int numBytes) {
+  void ensureCapacityForBytes(final UInt256 offset, final UInt256 numBytes) {
+    if (!offset.fitsInt()) return;
+    if (!numBytes.fitsInt()) return;
+    ensureCapacityForBytes(offset.intValue(), numBytes.intValue());
+  }
+
+  /**
+   * Expands the active words to accommodate the specified byte position.
+   *
+   * @param offset The location in memory to start with.
+   * @param numBytes The number of bytes to get.
+   */
+  void ensureCapacityForBytes(final int offset, final int numBytes) {
     // Do not increase the memory capacity if no bytes are being written
     // regardless of what the address may be.
     if (numBytes == 0) {
       return;
     }
-    final int lastByteIndex = Math.addExact(address, numBytes);
+    final int lastByteIndex = Math.addExact(offset, numBytes);
     final int lastWordRequired = ((lastByteIndex - 1) / Bytes32.SIZE);
     maybeExpandCapacity(lastWordRequired + 1);
   }

ethereum/core/src/main/java/org/hyperledger/besu/ethereum/vm/MessageFrame.java

@@ -550,7 +550,7 @@ public class MessageFrame {
    * @param offset The offset in memory
    * @param length The length of the memory access
    */
-  public void expandMemory(final int offset, final int length) {
+  public void expandMemory(final UInt256 offset, final UInt256 length) {
     memory.ensureCapacityForBytes(offset, length);
   }