Description

The action-handler system in Penumbra works by executing the checks in a sequential manner. The below demonstrates the execution of one transfer in N actions.

Action 1 -> check_stateless
Action 2 -> check_stateless
...
Action N -> check_stateless


Action 1 -> check_stateful
Action 2 -> check_stateful
...
Action N -> check_stateful


Action 1 -> execute
Action 2 -> execute
...
Action N -> execute

(See more about this in ref↗)

The issue arises when the side effects of the execution of an action invalidates an invariant that was checked for earlier.

Specifically, the validator::Definition action asserts that two validators with the same consensus key should not be uploaded, otherwise a tendermint could hang. This is done with the check_stateful implementation, which verifies that an existing validator with the same consensus key has not already been uploaded.

async fn check_stateful<S: StateRead + 'static>(&self, state: Arc<S>) -> Result<()> {
	...
	// Check whether the consensus key has already been used by another validator.
	if let Some(existing_v) = state
		.get_validator_by_consensus_key(&v.validator.consensus_key)
		.await?
	{
		if v.validator.identity_key != existing_v.identity_key {
			...
			// 2. If we submit a validator update to Tendermint that
			// includes duplicate consensus keys, Tendermint gets confused
			// and hangs.
			anyhow::bail!(
				"consensus key {:?} is already in use by validator {}",
				v.validator.consensus_key,
				existing_v.identity_key,
			);
		}
	}
}

A malicious attacker could submit a transaction with two validator::Definition actions; the check_stateful check will pass for both as no validator with that respective consensus key still exists. However, when the actions are executed, they will both upload validators with two of the same consensus key.

This is a time-of-check time-of-use bug ().

Impact

Two validators uploaded with the same consensus key could cause a tendermint hang; this eventually could halt the chain.

Recommendations

An error should be returned when multiple validator::Definition actions in a transaction are submitted.

Remediation

This issue has been acknowledged by Penumbra Labs, and fixes were implemented in the following commits:

• 1f084185↗
• 2949f8f0↗