Ensure validator indices in attester slashings are valid

ethereum/core/src/test/java/tech/pegasys/teku/core/fuzz/FuzzRegressionTest.java

@@ -0,0 +1,51 @@
+/*
+ * Copyright 2020 ConsenSys AG.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on
+ * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations under the License.
+ */
+
+package tech.pegasys.teku.core.fuzz;
+
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
+
+import com.google.common.io.Resources;
+import java.net.URL;
+import org.apache.tuweni.bytes.Bytes;
+import org.junit.jupiter.api.Test;
+import tech.pegasys.teku.core.BlockProcessorUtil;
+import tech.pegasys.teku.core.exceptions.BlockProcessingException;
+import tech.pegasys.teku.datastructures.operations.AttesterSlashing;
+import tech.pegasys.teku.datastructures.state.BeaconState;
+import tech.pegasys.teku.datastructures.state.BeaconStateImpl;
+import tech.pegasys.teku.datastructures.util.SimpleOffsetSerializer;
+import tech.pegasys.teku.ssz.SSZTypes.SSZList;
+
+public class FuzzRegressionTest {
+  @Test
+  void shouldRejectAttesterSlashingWithInvalidValidatorIndex() throws Exception {
+    final BeaconState state = load("issue2345/state.ssz", BeaconStateImpl.class);
+    final AttesterSlashing slashing =
+        load("issue2345/attester_slashing.ssz", AttesterSlashing.class);
+
+    assertThatThrownBy(
+            () ->
+                state.updated(
+                    mutableState ->
+                        BlockProcessorUtil.process_attester_slashings(
+                            mutableState, SSZList.singleton(slashing))))
+        .isInstanceOf(BlockProcessingException.class);
+  }
+
+  private <T> T load(final String resource, final Class<T> type) throws Exception {
+    final URL resourceUrl = FuzzRegressionTest.class.getResource(resource);
+    final Bytes data = Bytes.wrap(Resources.toByteArray(resourceUrl));
+    return SimpleOffsetSerializer.deserialize(data, type);
+  }
+}

ethereum/datastructures/src/main/java/tech/pegasys/teku/datastructures/util/AttestationUtil.java

@@ -14,6 +14,7 @@
 package tech.pegasys.teku.datastructures.util;
 
 import static com.google.common.base.Preconditions.checkArgument;
+import static java.util.stream.Collectors.toList;
 import static tech.pegasys.teku.datastructures.util.BeaconStateUtil.compute_epoch_at_slot;
 import static tech.pegasys.teku.datastructures.util.BeaconStateUtil.compute_signing_root;
 import static tech.pegasys.teku.datastructures.util.BeaconStateUtil.compute_start_slot_at_epoch;
@@ -29,7 +30,6 @@
 import java.util.HashSet;
 import java.util.List;
 import java.util.Set;
-import java.util.stream.Collectors;
 import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
 import org.apache.tuweni.bytes.Bytes;
@@ -91,10 +91,7 @@ public static IndexedAttestation get_indexed_attestation(
 
     return new IndexedAttestation(
         SSZList.createMutable(
-            attesting_indices.stream()
-                .sorted()
-                .map(UnsignedLong::valueOf)
-                .collect(Collectors.toList()),
+            attesting_indices.stream().sorted().map(UnsignedLong::valueOf).collect(toList()),
             MAX_VALIDATORS_PER_COMMITTEE,
             UnsignedLong.class),
         attestation.getData(),
@@ -150,13 +147,17 @@ public static AttestationProcessingResult is_valid_indexed_attestation(
     SSZList<UnsignedLong> indices = indexed_attestation.getAttesting_indices();
 
     List<UnsignedLong> bit_0_indices_sorted =
-        indices.stream().sorted().distinct().collect(Collectors.toList());
+        indices.stream().sorted().distinct().collect(toList());
     if (indices.isEmpty() || !indices.equals(bit_0_indices_sorted)) {
       return AttestationProcessingResult.invalid("Attesting indices are not sorted");
     }
 
     List<BLSPublicKey> pubkeys =
-        indices.stream().map(i -> getValidatorPubKey(state, i)).collect(Collectors.toList());
+        indices.stream().flatMap(i -> getValidatorPubKey(state, i).stream()).collect(toList());
+    if (pubkeys.size() < indices.size()) {
+      return AttestationProcessingResult.invalid(
+          "Attesting indices include non-existent validator");
+    }
 
     BLSSignature signature = indexed_attestation.getSignature();
     Bytes domain =

ethereum/datastructures/src/main/java/tech/pegasys/teku/datastructures/util/ValidatorsUtil.java

@@ -70,10 +70,16 @@ public static boolean is_eligible_for_activation(BeaconState state, Validator va
         && validator.getActivation_epoch().equals(Constants.FAR_FUTURE_EPOCH);
   }
 
-  public static BLSPublicKey getValidatorPubKey(BeaconState state, UnsignedLong validatorIndex) {
-    return BeaconStateCache.getTransitionCaches(state)
-        .getValidatorsPubKeys()
-        .get(validatorIndex, i -> state.getValidators().get(i.intValue()).getPubkey());
+  public static Optional<BLSPublicKey> getValidatorPubKey(
+      BeaconState state, UnsignedLong validatorIndex) {
+    if (state.getValidators().size() <= validatorIndex.longValue()
+        || validatorIndex.longValue() < 0) {
+      return Optional.empty();
+    }
+    return Optional.of(
+        BeaconStateCache.getTransitionCaches(state)
+            .getValidatorsPubKeys()
+            .get(validatorIndex, i -> state.getValidators().get(i.intValue()).getPubkey()));
   }
 
   /**

networking/eth2/src/main/java/tech/pegasys/teku/networking/eth2/gossip/topics/validation/BlockValidator.java

@@ -29,7 +29,6 @@
 import org.apache.logging.log4j.Logger;
 import org.apache.tuweni.bytes.Bytes;
 import tech.pegasys.teku.bls.BLS;
-import tech.pegasys.teku.bls.BLSPublicKey;
 import tech.pegasys.teku.bls.BLSSignature;
 import tech.pegasys.teku.core.StateTransition;
 import tech.pegasys.teku.core.exceptions.EpochProcessingException;
@@ -115,10 +114,10 @@ private boolean blockSignatureIsValidWithRespectToProposerIndex(
     final Bytes signing_root = compute_signing_root(block.getMessage(), domain);
     final BLSSignature signature = block.getSignature();
 
-    BLSPublicKey proposerPubkey =
-        ValidatorsUtil.getValidatorPubKey(postState, block.getMessage().getProposer_index());
-
-    boolean signatureValid = BLS.verify(proposerPubkey, signing_root, signature);
+    boolean signatureValid =
+        ValidatorsUtil.getValidatorPubKey(postState, block.getMessage().getProposer_index())
+            .map(publicKey -> BLS.verify(publicKey, signing_root, signature))
+            .orElse(false);
 
     return signatureValid && receivedValidBlockInfoSet.add(new SlotAndProposer(block));
   }

networking/eth2/src/main/java/tech/pegasys/teku/networking/eth2/gossip/topics/validation/SignedAggregateAndProofValidator.java

@@ -104,11 +104,15 @@ public InternalValidationResult validate(final ValidateableAttestation attestati
       return SAVE_FOR_FUTURE;
     }
     final BeaconState state = maybeState.get();
-    final BLSPublicKey aggregatorPublicKey =
+    final Optional<BLSPublicKey> aggregatorPublicKey =
         ValidatorsUtil.getValidatorPubKey(state, aggregateAndProof.getIndex());
+    if (aggregatorPublicKey.isEmpty()) {
+      LOG.trace("Rejecting aggregate with invalid index");
+      return REJECT;
+    }
 
     if (!isSelectionProofValid(
-        aggregateSlot, state, aggregatorPublicKey, aggregateAndProof.getSelection_proof())) {
+        aggregateSlot, state, aggregatorPublicKey.get(), aggregateAndProof.getSelection_proof())) {
       LOG.trace("Rejecting aggregate with incorrect selection proof");
       return REJECT;
     }
@@ -129,7 +133,7 @@ public InternalValidationResult validate(final ValidateableAttestation attestati
       return REJECT;
     }
 
-    if (!isSignatureValid(signedAggregate, state, aggregatorPublicKey)) {
+    if (!isSignatureValid(signedAggregate, state, aggregatorPublicKey.get())) {
       LOG.trace("Rejecting aggregate with invalid signature");
       return REJECT;
     }