Correct the prior documented TOCTOU

Now, if a malicious validator set publishes a malicious `Batch` at the last
moment, it'll cause all future `Batch`s signed by the next validator set to
require a bool being set (yet they never will set it).

This will prevent the handover.

The only overhead is having two distinct `batch_message` calls on-chain.
This commit is contained in:
Luke Parker
2023-10-13 04:40:59 -04:00
parent e6aa9df428
commit d50fe87801
8 changed files with 45 additions and 21 deletions

View File

@@ -39,7 +39,8 @@ pub async fn provide_batch(batch: Batch) -> [u8; 32] {
let block = publish_tx(&Serai::execute_batch(SignedBatch {
batch: batch.clone(),
signature: pair.sign(&batch_message(&batch)),
// TODO: This `batch.id == 0` line only works when session == 0
signature: pair.sign(&batch_message(batch.id == 0, &batch)),
}))
.await;

View File

@@ -155,14 +155,21 @@ pub mod pallet {
// verify the signature
let network = batch.batch.network;
let (current_session, prior, current) = keys_for_network::<T>(network)?;
let batch_message = batch_message(&batch.batch);
// Check the prior key first since only a single `Batch` (the last one) will be when prior is
// Some yet prior wasn't the signing key
let valid_by_prior =
if let Some(key) = prior { key.verify(&batch_message, &batch.signature) } else { false };
let valid_by_prior = if let Some(key) = prior {
key.verify(&batch_message(false, &batch.batch), &batch.signature)
} else {
false
};
let valid = valid_by_prior ||
(if let Some(key) = current {
key.verify(&batch_message, &batch.signature)
key.verify(
// This `== 0` is valid as either it'll be the first Batch for the first set, or if
// they never had a Batch, the first Batch for the next set
&batch_message((batch.batch.id == 0) || prior.is_some(), &batch.batch),
&batch.signature,
)
} else {
false
});

View File

@@ -84,6 +84,6 @@ impl Zeroize for SignedBatch {
// TODO: Make this an associated method?
/// The message for the batch signature.
pub fn batch_message(batch: &Batch) -> Vec<u8> {
[b"InInstructions-batch".as_ref(), &batch.encode()].concat()
pub fn batch_message(is_first_batch_of_set: bool, batch: &Batch) -> Vec<u8> {
[b"InInstructions-batch".as_ref(), &(is_first_batch_of_set, batch).encode()].concat()
}