From 8416203ccb3128ea996baaf6500b908d212be50c Mon Sep 17 00:00:00 2001 From: Vladimir Guguiev <1524432+vovacodes@users.noreply.github.com> Date: Fri, 23 Jun 2023 11:42:18 +0200 Subject: [PATCH] fix(execution): prevent reentrancy attacks with cancelling proposal during execution --- .../src/instructions/batch_execute_transaction.rs | 8 ++++++++ .../src/instructions/vault_transaction_execute.rs | 3 +++ programs/multisig/src/state/proposal.rs | 2 ++ sdk/multisig/idl/multisig.json | 3 +++ sdk/multisig/src/generated/types/ProposalStatus.ts | 5 +++++ 5 files changed, 21 insertions(+) diff --git a/programs/multisig/src/instructions/batch_execute_transaction.rs b/programs/multisig/src/instructions/batch_execute_transaction.rs index a67c5fe6..dec17aa7 100644 --- a/programs/multisig/src/instructions/batch_execute_transaction.rs +++ b/programs/multisig/src/instructions/batch_execute_transaction.rs @@ -1,4 +1,5 @@ use anchor_lang::prelude::*; +use std::borrow::Borrow; use crate::errors::*; use crate::state::*; @@ -146,6 +147,10 @@ impl BatchExecuteTransaction<'_> { &ephemeral_signer_keys, )?; + let current_status = proposal.status.clone(); + // Set the proposal state to Executing to prevent reentrancy attacks (e.g. cancelling proposal) in the middle of execution. + proposal.status = ProposalStatus::Executing; + // Execute the transaction message instructions one-by-one. executable_message.execute_message( &vault_seeds @@ -155,6 +160,9 @@ impl BatchExecuteTransaction<'_> { &ephemeral_signer_seeds, )?; + // Restore the proposal status after execution. + proposal.status = current_status; + // Increment the executed transaction index. batch.executed_transaction_index = batch .executed_transaction_index diff --git a/programs/multisig/src/instructions/vault_transaction_execute.rs b/programs/multisig/src/instructions/vault_transaction_execute.rs index 8ad2047b..044e6ab4 100644 --- a/programs/multisig/src/instructions/vault_transaction_execute.rs +++ b/programs/multisig/src/instructions/vault_transaction_execute.rs @@ -128,6 +128,9 @@ impl VaultTransactionExecute<'_> { &ephemeral_signer_keys, )?; + // Set the proposal state to Executing to prevent reentrancy attacks (e.g. cancelling proposal) in the middle of execution. + proposal.status = ProposalStatus::Executing; + // Execute the transaction message instructions one-by-one. executable_message.execute_message( &vault_seeds diff --git a/programs/multisig/src/state/proposal.rs b/programs/multisig/src/state/proposal.rs index 9e76d298..ec757a86 100644 --- a/programs/multisig/src/state/proposal.rs +++ b/programs/multisig/src/state/proposal.rs @@ -136,6 +136,8 @@ pub enum ProposalStatus { Rejected { timestamp: i64 }, /// Proposal has been approved and is pending execution. Approved { timestamp: i64 }, + /// Proposal is being executed. This is a transient state that always transitions to `Executed` in the span of a single transaction. + Executing, /// Proposal has been executed. Executed { timestamp: i64 }, /// Proposal has been cancelled. diff --git a/sdk/multisig/idl/multisig.json b/sdk/multisig/idl/multisig.json index f9b93981..49c06f1e 100644 --- a/sdk/multisig/idl/multisig.json +++ b/sdk/multisig/idl/multisig.json @@ -1988,6 +1988,9 @@ } ] }, + { + "name": "Executing" + }, { "name": "Executed", "fields": [ diff --git a/sdk/multisig/src/generated/types/ProposalStatus.ts b/sdk/multisig/src/generated/types/ProposalStatus.ts index 1c5587fe..0259940c 100644 --- a/sdk/multisig/src/generated/types/ProposalStatus.ts +++ b/sdk/multisig/src/generated/types/ProposalStatus.ts @@ -20,6 +20,7 @@ export type ProposalStatusRecord = { Active: { timestamp: beet.bignum } Rejected: { timestamp: beet.bignum } Approved: { timestamp: beet.bignum } + Executing: void /* scalar variant */ Executed: { timestamp: beet.bignum } Cancelled: { timestamp: beet.bignum } } @@ -49,6 +50,9 @@ export const isProposalStatusRejected = ( export const isProposalStatusApproved = ( x: ProposalStatus ): x is ProposalStatus & { __kind: 'Approved' } => x.__kind === 'Approved' +export const isProposalStatusExecuting = ( + x: ProposalStatus +): x is ProposalStatus & { __kind: 'Executing' } => x.__kind === 'Executing' export const isProposalStatusExecuted = ( x: ProposalStatus ): x is ProposalStatus & { __kind: 'Executed' } => x.__kind === 'Executed' @@ -92,6 +96,7 @@ export const proposalStatusBeet = beet.dataEnum([ 'ProposalStatusRecord["Approved"]' ), ], + ['Executing', beet.unit], [ 'Executed',