Repository: solana-labs/solana-program-library
Pull request #6863 (2024-06-17 18:00:33)

token-2022: [L-01] Check for all possible self-auth with CPI guard
#### Problem

Although #6724 fixed self-delegated transfers with CPI guard, it didn't fix all situations, including if the permanent delegate is the owner. And it also didn't cover burns. Per Certora's audit report:

> Description: Fund decreasing instructions (Transfer, TransferChecked, TransferCheckedWithFee, and Burn) when protected by CpiGuard should not be allowed if signed by the owner. The intention is that they are only allowed if signed by some other account delegate. This property is violated when
> * an account is delegated to itself
> * the account is a permanent delegate of the mint
> * the account is owned by system program or incinerator (in this case, signing authority is not checked and can be arbitrary) 

> Recommendation: Revert instruction when it is called in CPI, with enabled CpiGuard, and signed by the source account owner.

#### Solution

Implement the recommendation by generalizing the CPI Guard check.


Patch:
diff --git a/token/program-2022/src/processor.rs b/token/program-2022/src/processor.rs
index 8ea190487..88d8c1178 100644
--- a/token/program-2022/src/processor.rs
+++ b/token/program-2022/src/processor.rs
@@ -381,6 +381,18 @@ impl Processor {
         }
 
         let self_transfer = source_account_info.key == destination_account_info.key;
+        if let Ok(cpi_guard) = source_account.get_extension::<CpiGuard>() {
+            // Blocks all cases where the authority has signed if CPI Guard is
+            // enabled, including:
+            // * the account is delegated to the owner
+            // * the account owner is the permanent delegate
+            if *authority_info.key == source_account.base.owner
+                && cpi_guard.lock_cpi.into()
+                && in_cpi()
+            {
+                return Err(TokenError::CpiGuardTransferBlocked.into());
+            }
+        }
         match (source_account.base.delegate, maybe_permanent_delegate) {
             (_, Some(ref delegate)) if authority_info.key == delegate => Self::validate_owner(
                 program_id,
@@ -403,15 +415,6 @@ impl Processor {
                     authority_info_data_len,
                     account_info_iter.as_slice(),
                 )?;
-                if let Ok(cpi_guard) = source_account.get_extension::<CpiGuard>() {
-                    // If delegated to self, don't allow a transfer with CPI Guard
-                    if delegate == source_account.base.owner
-                        && cpi_guard.lock_cpi.into()
-                        && in_cpi()
-                    {
-                        return Err(TokenError::CpiGuardTransferBlocked.into());
-                    }
-                }
                 let delegated_amount = u64::from(source_account.base.delegated_amount);
                 if delegated_amount < amount {
                     return Err(TokenError::InsufficientFunds.into());
@@ -434,12 +437,6 @@ impl Processor {
                     authority_info_data_len,
                     account_info_iter.as_slice(),
                 )?;
-
-                if let Ok(cpi_guard) = source_account.get_extension::<CpiGuard>() {
-                    if cpi_guard.lock_cpi.into() && in_cpi() {
-                        return Err(TokenError::CpiGuardTransferBlocked.into());
-                    }
-                }
             }
         }
 
@@ -1033,6 +1030,19 @@ impl Processor {
         }
         let maybe_permanent_delegate = get_permanent_delegate(&mint);
 
+        if let Ok(cpi_guard) = source_account.get_extension::<CpiGuard>() {
+            // Blocks all cases where the authority has signed if CPI Guard is
+            // enabled, including:
+            // * the account is delegated to the owner
+            // * the account owner is the permanent delegate
+            if *authority_info.key == source_account.base.owner
+                && cpi_guard.lock_cpi.into()
+                && in_cpi()
+            {
+                return Err(TokenError::CpiGuardBurnBlocked.into());
+            }
+        }
+
         if !source_account
             .base
             .is_owned_by_system_program_or_incinerator()
@@ -1080,12 +1090,6 @@ impl Processor {
                         authority_info_data_len,
                         account_info_iter.as_slice(),
                     )?;
-
-                    if let Ok(cpi_guard) = source_account.get_extension::<CpiGuard>() {
-                        if cpi_guard.lock_cpi.into() && in_cpi() {
-                            return Err(TokenError::CpiGuardBurnBlocked.into());
-                        }
-                    }
                 }
             }
         }


Test patch:
diff --git a/token/program-2022-test/tests/cpi_guard.rs b/token/program-2022-test/tests/cpi_guard.rs
index 5a6591afb..16b3ab754 100644
--- a/token/program-2022-test/tests/cpi_guard.rs
+++ b/token/program-2022-test/tests/cpi_guard.rs
@@ -420,6 +420,27 @@ async fn test_cpi_guard_burn() {
         let alice_state = token_obj.get_account_info(&alice.pubkey()).await.unwrap();
         assert_eq!(alice_state.base.amount, amount);
 
+        // delegate-auth cpi burn by self does not work
+        token_obj
+            .approve(
+                &alice.pubkey(),
+                &alice.pubkey(),
+                &alice.pubkey(),
+                1,
+                &[&alice],
+            )
+            .await
+            .unwrap();
+
+        let error = token_obj
+            .process_ixs(&[mk_burn(alice.pubkey(), do_checked)], &[&alice])
+            .await
+            .unwrap_err();
+        assert_eq!(error, client_error(TokenError::CpiGuardBurnBlocked));
+
+        let alice_state = token_obj.get_account_info(&alice.pubkey()).await.unwrap();
+        assert_eq!(alice_state.base.amount, amount);
+
         // delegate-auth cpi burn with cpi guard works
         token_obj
             .approve(