From 2405060a7e07c7a7141ba6f45da54eb9d9c21fa4 Mon Sep 17 00:00:00 2001 From: protolambda Date: Thu, 12 Dec 2019 01:45:59 +0100 Subject: [PATCH 1/2] Fixes #1486: disallow duplicate indices in indexed attestation --- specs/core/0_beacon-chain.md | 4 +- .../test_process_attester_slashing.py | 60 +++++++++++++++++++ 2 files changed, 62 insertions(+), 2 deletions(-) diff --git a/specs/core/0_beacon-chain.md b/specs/core/0_beacon-chain.md index 7ce93eb6f..8234c9324 100644 --- a/specs/core/0_beacon-chain.md +++ b/specs/core/0_beacon-chain.md @@ -628,8 +628,8 @@ def is_valid_indexed_attestation(state: BeaconState, indexed_attestation: Indexe # Verify max number of indices if not len(indices) <= MAX_VALIDATORS_PER_COMMITTEE: return False - # Verify indices are sorted - if not indices == sorted(indices): + # Verify indices are sorted and unique + if not indices == sorted(set(indices)): return False # Verify aggregate signature if not bls_verify( diff --git a/test_libs/pyspec/eth2spec/test/phase_0/block_processing/test_process_attester_slashing.py b/test_libs/pyspec/eth2spec/test/phase_0/block_processing/test_process_attester_slashing.py index 85e807ec0..3fc2f8a38 100644 --- a/test_libs/pyspec/eth2spec/test/phase_0/block_processing/test_process_attester_slashing.py +++ b/test_libs/pyspec/eth2spec/test/phase_0/block_processing/test_process_attester_slashing.py @@ -252,6 +252,66 @@ def test_att2_bad_replaced_index(spec, state): yield from run_attester_slashing_processing(spec, state, attester_slashing, False) +@with_all_phases +@spec_state_test +@always_bls +def test_att1_duplicate_index_normal_signed(spec, state): + attester_slashing = get_valid_attester_slashing(spec, state, signed_1=False, signed_2=True) + + indices = attester_slashing.attestation_1.attesting_indices + indices.pop(1) # remove an index, make room for the additional duplicate index. + indices.append(indices[0]) # add one of the indices a second time + attester_slashing.attestation_1.attesting_indices = sorted(indices) + sign_indexed_attestation(spec, state, attester_slashing.attestation_1) + # it will just appear normal, unless the double index is spotted + yield from run_attester_slashing_processing(spec, state, attester_slashing, False) + + +@with_all_phases +@spec_state_test +@always_bls +def test_att2_duplicate_index_normal_signed(spec, state): + attester_slashing = get_valid_attester_slashing(spec, state, signed_1=True, signed_2=False) + + indices = attester_slashing.attestation_2.attesting_indices + indices.pop(2) # remove an index, make room for the additional duplicate index. + indices.append(indices[1]) # add one of the indices a second time + attester_slashing.attestation_2.attesting_indices = sorted(indices) + sign_indexed_attestation(spec, state, attester_slashing.attestation_2) + # it will just appear normal, unless the double index is spotted + yield from run_attester_slashing_processing(spec, state, attester_slashing, False) + + +@with_all_phases +@spec_state_test +@always_bls +def test_att1_duplicate_index_double_signed(spec, state): + attester_slashing = get_valid_attester_slashing(spec, state, signed_1=False, signed_2=True) + + indices = attester_slashing.attestation_1.attesting_indices + indices.pop(1) # remove an index, make room for the additional duplicate index. + indices.append(indices[2]) # add one of the indices a second time + attester_slashing.attestation_1.attesting_indices = sorted(indices) + sign_indexed_attestation(spec, state, attester_slashing.attestation_1) # will have one attester signing it double + + yield from run_attester_slashing_processing(spec, state, attester_slashing, False) + + +@with_all_phases +@spec_state_test +@always_bls +def test_att2_duplicate_index_double_signed(spec, state): + attester_slashing = get_valid_attester_slashing(spec, state, signed_1=True, signed_2=False) + + indices = attester_slashing.attestation_2.attesting_indices + indices.pop(1) # remove an index, make room for the additional duplicate index. + indices.append(indices[2]) # add one of the indices a second time + attester_slashing.attestation_2.attesting_indices = sorted(indices) + sign_indexed_attestation(spec, state, attester_slashing.attestation_2) # will have one attester signing it double + + yield from run_attester_slashing_processing(spec, state, attester_slashing, False) + + @with_all_phases @spec_state_test def test_unsorted_att_1(spec, state): From 5c26d8e52fd48833ed22e4773915cc9b2a3b248b Mon Sep 17 00:00:00 2001 From: protolambda Date: Thu, 12 Dec 2019 16:29:30 +0100 Subject: [PATCH 2/2] fix normal signed case; only sign for 1 of the duplicate indices --- .../test_process_attester_slashing.py | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/test_libs/pyspec/eth2spec/test/phase_0/block_processing/test_process_attester_slashing.py b/test_libs/pyspec/eth2spec/test/phase_0/block_processing/test_process_attester_slashing.py index 3fc2f8a38..98a6e25e5 100644 --- a/test_libs/pyspec/eth2spec/test/phase_0/block_processing/test_process_attester_slashing.py +++ b/test_libs/pyspec/eth2spec/test/phase_0/block_processing/test_process_attester_slashing.py @@ -260,9 +260,14 @@ def test_att1_duplicate_index_normal_signed(spec, state): indices = attester_slashing.attestation_1.attesting_indices indices.pop(1) # remove an index, make room for the additional duplicate index. + attester_slashing.attestation_1.attesting_indices = sorted(indices) + + # sign it, the signature will be valid for a single occurence. If the transition accidentally ignores the duplicate. + sign_indexed_attestation(spec, state, attester_slashing.attestation_1) + indices.append(indices[0]) # add one of the indices a second time attester_slashing.attestation_1.attesting_indices = sorted(indices) - sign_indexed_attestation(spec, state, attester_slashing.attestation_1) + # it will just appear normal, unless the double index is spotted yield from run_attester_slashing_processing(spec, state, attester_slashing, False) @@ -275,9 +280,14 @@ def test_att2_duplicate_index_normal_signed(spec, state): indices = attester_slashing.attestation_2.attesting_indices indices.pop(2) # remove an index, make room for the additional duplicate index. + attester_slashing.attestation_2.attesting_indices = sorted(indices) + + # sign it, the signature will be valid for a single occurence. If the transition accidentally ignores the duplicate. + sign_indexed_attestation(spec, state, attester_slashing.attestation_2) + indices.append(indices[1]) # add one of the indices a second time attester_slashing.attestation_2.attesting_indices = sorted(indices) - sign_indexed_attestation(spec, state, attester_slashing.attestation_2) + # it will just appear normal, unless the double index is spotted yield from run_attester_slashing_processing(spec, state, attester_slashing, False)