Malicious validator can trigger epoch
Description
When a validator transitions out of the Active
state, a flag is set to indicate that the current epoch should be ended after the current block has been processed:
async fn set_validator_state_inner(
&mut self,
identity_key: &IdentityKey,
old_state: validator::State,
new_state: validator::State,
) -> Result<()> {
use validator::State::*;
let validator_state_path = state_key::validators::state::by_id(identity_key);
// Validator state transitions are usually triggered by an epoch transition. The exception
// to this rule is when a validator exits the active set. In this case, we want to end the
// current epoch early in order to hold that validator transitions happen at epoch boundaries.
if let (Active, Defined | Disabled | Jailed | Tombstoned) = (old_state, new_state) {
self.set_end_epoch_flag();
}
The issue is that it is possible for a currently bonded validator to disable and enable themselves in the same transaction, which will end up triggering the new epoch and keep the validator in the active set.
The first action will change the validator from Active
to Disabled
, and the second action will change the validator from Disabled
to Inactive
. The end_epoch
handler will then be run at the end of the block, and set_active_and_inactive_validators
will set the inactive validator back to Active
:
for (v, _) in active {
self.set_validator_state(v, validator::State::Active)
.await?;
}
for (v, _) in inactive {
self.set_validator_state(v, validator::State::Inactive)
.await?;
}
Ok(())
Impact
A malicious bonded validator could cause a new epoch to happen every block. During our testing, we noticed around a 20% slowdown in block production when this was happening. The epoch number is also stored as a u16
in the tct::Position
, so it could be possible for this to overflow.
Recommendations
A validator should not be able to disable and enable themselves in the same transaction, or the current bonding state should be checked to ensure that it is not currently unbonding.
Remediation
This issue has been acknowledged by Penumbra Labs, and a fix was implemented in commit 349c0baf↗.