Closed parithosh closed 4 years ago
parithosh
commented
4 years ago I just noticed that the stacktrace has a statement couldn't interpret ETH2FUZZ_BEACONSTATE: environment variable not found. I can't find any info on setting this variable in the README, please do let me know how one can set the variable. The issue might then just be a config related error and not a fuzzing crash.
gnattishness
commented
4 years ago Thanks for your efforts @parithosh!
Agreed there doesn't look to be any info on ETH2FUZZ_BEACONSTATE in the current README.
From the Makefile, we can see that it's set to point to eth2fuzz corpora's folder of known beaconstates:
https://github.com/sigp/beacon-fuzz/blob/4e329051eabb8ed7b50beee8a6ff71fa70c6b585/beaconfuzz_v2/Makefile#L53-L54
But this would only be a config issue during the attempt to reproduce, and not during the initial crash identification. We'll look into it further!
pventuzelo
commented
4 years ago @gnattishness I confirm, when ETH2FUZZ_BEACONSTATE is not set, I make the program to panic ;)
Thanks @parithosh
parithosh
commented
4 years ago @pventuzelo glad that it was a config error then :D My other issue might have the same cause, so I think you can just add the same comment and close that as well.
pventuzelo
commented
4 years ago I reopen the issue until we give a try to your testcase, look like the variable was not set only during the repro
pventuzelo
commented
4 years ago With the latest update of prysm and beaconfuzz_v2, i'm not able to repro the bug using this command:
ETH2FUZZ_BEACONSTATE=../eth2fuzz/workspace/corpora/beaconstate cargo +nightly fuzz run struct_attester_slashing crash-5f5801ee2ad7be3a2c9017f3c731ccdbd3b0e64cor using:
./beaconfuzz_v2 debugLook like my environment was not correct during the previous test, i'm reopening this issue:
My command for testing:
ASAN_OPTIONS=detect_leaks=0 RUSTFLAGS='-L /home/scop/Documents/consulting/sigmaprime/prysm/pfuzz/ -L /home/scop/Documents/consulting/sigmaprime/nim-beacon-state/build/ ' ETH2FUZZ_BEACONSTATE=../eth2fuzz/workspace/corpora/beaconstate cargo +nightly fuzz run struct_attester_slashing crash-5f5801ee2ad7be3a2c9017f3c731ccdbd3b0e64cother command:
./beaconfuzz_v2 debug nimbus_attester_slashing_issue_61/pre.ssz nimbus_attester_slashing_issue_61/attslash.ssz attesterslashingResult:
thread '<unnamed>' panicked at 'assertion failed: `(left == right)`
left: `false`,
right: `true`', /home/scop/Documents/consulting/sigmaprime/beacon-fuzz/beaconfuzz_v2/libs/eth2clientsfuzz/src/attester_slashing.rs:39:17Meaning: nimbus is not agree with prysm and lighthouse
cc @zedt3ster
zedt3ster
commented
4 years ago Thanks for reporting @parithosh - this is another great find by the structural fuzzer, highlighting a subtle difference between how Nimbus, Prysm and Lighthouse handle the AttesterSlashing processing.
As per the eth2 specification:
def process_attester_slashing(state: BeaconState, attester_slashing: AttesterSlashing) -> None:
attestation_1 = attester_slashing.attestation_1
attestation_2 = attester_slashing.attestation_2
assert is_slashable_attestation_data(attestation_1.data, attestation_2.data)
assert is_valid_indexed_attestation(state, attestation_1)
assert is_valid_indexed_attestation(state, attestation_2)
slashed_any = False
indices = set(attestation_1.attesting_indices).intersection(attestation_2.attesting_indices)
for index in sorted(indices):
if is_slashable_validator(state.validators[index], get_current_epoch(state)):
slash_validator(state, index)
slashed_any = True
assert slashed_anyThe function is_valid_indexed_attestation is defined as follows:
def is_valid_indexed_attestation(state: BeaconState, indexed_attestation: IndexedAttestation) -> bool:
"""
Check if ``indexed_attestation`` is not empty, has sorted and unique indices and has a valid aggregate signature.
"""
# Verify indices are sorted and unique
indices = indexed_attestation.attesting_indices
if len(indices) == 0 or not indices == sorted(set(indices)):
return False
# Verify aggregate signature
pubkeys = [state.validators[i].pubkey for i in indices]
domain = get_domain(state, DOMAIN_BEACON_ATTESTER, indexed_attestation.data.target.epoch)
signing_root = compute_signing_root(indexed_attestation.data, domain)
return bls.FastAggregateVerify(pubkeys, signing_root, indexed_attestation.signature)It turns out that Lighthouse and Prysm are performing the indexed attestation validation as part of their signature verification:
In Lighthouse:
The closure |i| get_pubkey_from_state(state, i) would return an error for out-of-range attesting indices, however since fuzzing disables BLS verification, that particular check is not performed (the entire code block is skipped).
For Prysm:
When BLS signatures are disabled, PublicKeyFromBytes() returns an empty BLS signatures, regardless of the attesting indices provided (therefore not checking for out-of-range attesting indices).
As a result, the AttesterSlashing object produced by the structural fuzzer is considered valid by Lighthouse and Prysm, with the associated state transition resulting in a post-BeaconState.
For Nimbus:
In the snippet linked above, we can see that Nimbus performs an additional check on the attesting indices:
let num_validators = state.validators.lenu64
if anyIt(indexed_attestation.attesting_indices, it >= num_validators):
return err("indexed attestation: not all indices valid validators")As a result, the AttesterSlashing generated by the structural fuzzer is rejected by Nimbus (since the second attesting index in the second IndexedAttestation causes the check above to fail).
This explains the discrepancy raised by the fuzzer. Thanks again for reporting this @parithosh !
parithosh
commented
4 years ago Thanks for the details response @zedt3ster ! Glad I could be of help :)
I've identified a fuzzer crash and am contributing to the security of Ethereum 2!
I've done and provided the following:
[FUZZ]issue already refers to that crasherbeacon-fuzzversion or commit used.Info to Reproduce
make fuzz_attester_slashing-structcrash-5f5801ee2ad7be3a2c9017f3c731ccdbd3b0e64ccrash-5f5801ee2ad7be3a2c9017f3c731ccdbd3b0e64c.zipCrash output and stacktrace
Attached as a screenshot:
Re-ran the input on the nightly compiler (
rustup default nightly, it refused to compile ondefault). Command used to re-run input:RUST_BACKTRACE=1 cargo fuzz run struct_attester_slashing fuzz/artifacts/struct_attester_slashing/crash-5f5801ee2ad7be3a2c9017f3c731ccdbd3b0e64cStacktrace:
Your Environment
b56bf452adc05ac5b0a73aa8844dac0465894ebc refs/heads/beaconfuzz_v2