fix and improve signer-auth.md & reinitialization-attacks.md & pda-sharing.md & owner-checks.md & closing-accounts.md & arbitrary-cpi.md

content/courses/program-security/arbitrary-cpi.md

@@ -128,9 +128,8 @@ crate provides the address of the SPL Token Program.
 ### Use an Anchor CPI module
 
 A simpler way to manage program checks is to use Anchor CPI modules. We learned
-in a
-[previous lesson](https://github.com/Unboxed-Software/solana-course/blob/main/content/anchor-cpi)
-that Anchor can automatically generate CPI modules to make CPIs into the program
+in a [previous lesson](/content/courses/onchain-development/anchor-cpi.md) that
+Anchor can automatically generate CPI modules to make CPIs into the program
 simpler. These modules also enhance security by verifying the public key of the
 program that’s passed into one of its public instructions.
 
@@ -256,7 +255,7 @@ There is already a test in the `tests` directory for this. It's long, but take a
 minute to look at it before we talk through it together:
 
 ```typescript
-it("Insecure instructions allow attacker to win every time", async () => {
+it("Insecure instructions allow attacker to win every time successfully", async () => {
   // Initialize player one with real metadata program
   await gameplayProgram.methods
     .createCharacterInsecure()
@@ -352,7 +351,7 @@ pub struct CreateCharacterSecure<'info> {
     #[account(
         init,
         payer = authority,
-        space = 8 + 32 + 32 + 64,
+        space = DISCRIMINATOR_SIZE + Character::INIT_SPACE,
         seeds = [authority.key().as_ref()],
         bump
     )]
@@ -404,12 +403,12 @@ new test. This test just needs to attempt to initialize the attacker's character
 and expect an error to be thrown.
 
 ```typescript
-it("Secure character creation doesn't allow fake program", async () => {
+it("Secure character creation with fake program should throw an exception", async () => {
   try {
     await gameplayProgram.methods
       .createCharacterSecure()
       .accounts({
-        metadataProgram: fakeMetadataProgram.programId,
+        metadataProgram: fakeMetadataProgram.programId, // despite the compile error on this line.
         authority: attacker.publicKey,
       })
       .signers([attacker])

content/courses/program-security/closing-accounts.md

@@ -18,8 +18,7 @@ description:
   exempt. Closing accounts involves transferring the lamports stored in the
   account for rent exemption to another account of your choosing.
 - You can use the Anchor `#[account(close = <address_to_send_lamports>)]`
-  constraint to securely close accounts and set the account discriminator to the
-  `CLOSED_ACCOUNT_DISCRIMINATOR`
+  constraint to securely close accounts.
   ```rust
   #[account(mut, close = receiver)]
   pub data_account: Account<'info, MyData>,
@@ -96,66 +95,47 @@ the program and even drain a protocol.
 
 ### Secure account closing
 
-The two most important things you can do to close this loophole are to zero out
-the account data and add an account discriminator that represents the account
-has been closed. You need _both_ of these things to avoid unintended program
-behavior.
-
-An account with zeroed out data can still be used for some things, especially if
-it's a PDA whose address derivation is used within the program for verification
-purposes. However, the damage may be potentially limited if the attacker can't
-access the previously-stored data.
-
-To further secure the program, however, closed accounts should be given an
-account discriminator that designates it as "closed," and all instructions
-should perform checks on all passed-in accounts that return an error if the
-account is marked closed.
+The two most important things you can do to close this loophole are to change
+the account's ownership and reallocate the size of the account's data with 0
+bytes.
 
 Look at the example below. This program transfers the lamports out of an
-account, zeroes out the account data, and sets an account discriminator in a
-single instruction in hopes of preventing a subsequent instruction from
-utilizing this account again before it has been garbage collected. Failing to do
-any one of these things would result in a security vulnerability.
+account, changes the account's ownership to the system program, and reallocates
+the size of the account's data with 0 bytes in hopes of preventing a subsequent
+instruction from utilizing th is account again before it has been garbage
+collected. Failing to do any one of these things would result in a security
+vulnerability.
 
 ```rust
 use anchor_lang::prelude::*;
-use std::io::Write;
-use std::ops::DerefMut;
 
 declare_id!("Fg6PaFpoGXkYsidMpWTK6W2BeZ7FEfcYkg476zPFsLnS");
 
 #[program]
-pub mod closing_accounts_insecure_still_still {
+pub mod closing_accounts_insecure {
     use super::*;
+    use anchor_lang::solana_program::system_program;
 
-    pub fn close(ctx: Context<Close>) -> ProgramResult {
-        let account = ctx.accounts.account.to_account_info();
 
+    pub fn close(ctx: Context<Close>) -> ProgramResult {
         let dest_starting_lamports = ctx.accounts.destination.lamports();
+        let account_to_close = tx.accounts.lottery_entry.to_account_info();
 
         **ctx.accounts.destination.lamports.borrow_mut() = dest_starting_lamports
-            .checked_add(account.lamports())
+            .checked_add(account_to_close.lamports())
             .unwrap();
-        **account.lamports.borrow_mut() = 0;
+        **account_to_close.lamports.borrow_mut() = 0;
 
-        let mut data = account.try_borrow_mut_data()?;
-        for byte in data.deref_mut().iter_mut() {
-            *byte = 0;
-        }
-
-        let dst: &mut [u8] = &mut data;
-        let mut cursor = std::io::Cursor::new(dst);
-        cursor
-            .write_all(&anchor_lang::__private::CLOSED_ACCOUNT_DISCRIMINATOR)
-            .unwrap();
+        account_to_close.assign(&system_program::ID);
+        account_to_close.realloc(0, false)?;
 
         Ok(())
     }
 }
 
 #[derive(Accounts)]
 pub struct Close<'info> {
-    account: Account<'info, Data>,
+    account_to_close: Account<'info, Data>,
     destination: AccountInfo<'info>,
 }
 
@@ -165,73 +145,6 @@ pub struct Data {
 }
 ```
 
-Note that the example above is using Anchor's `CLOSED_ACCOUNT_DISCRIMINATOR`.
-This is simply an account discriminator where each byte is `255`. The
-discriminator doesn't have any inherent meaning, but if you couple it with
-account validation checks that return errors any time an account with this
-discriminator is passed to an instruction, you'll stop your program from
-unintentionally processing an instruction with a closed account.
-
-#### Manual Force Defund
-
-There is still one small issue. While the practice of zeroing out account data
-and adding a "closed" account discriminator will stop your program from being
-exploited, a user can still keep an account from being garbage collected by
-refunding the account's lamports before the end of an instruction. This results
-in one or potentially many accounts existing in a limbo state where they cannot
-be used but also cannot be garbage collected.
-
-To handle this edge case, you may consider adding an instruction that will allow
-_anyone_ to defund accounts tagged with the "closed" account discriminator. The
-only account validation this instruction would perform is to ensure that the
-account being defunded is marked as closed. It may look something like this:
-
-```rust
-use anchor_lang::__private::CLOSED_ACCOUNT_DISCRIMINATOR;
-use anchor_lang::prelude::*;
-use std::io::{Cursor, Write};
-use std::ops::DerefMut;
-
-...
-
-    pub fn force_defund(ctx: Context<ForceDefund>) -> ProgramResult {
-        let account = &ctx.accounts.account;
-
-        let data = account.try_borrow_data()?;
-        assert!(data.len() > 8);
-
-        let mut discriminator = [0u8; 8];
-        discriminator.copy_from_slice(&data[0..8]);
-        if discriminator != CLOSED_ACCOUNT_DISCRIMINATOR {
-            return Err(ProgramError::InvalidAccountData);
-        }
-
-        let dest_starting_lamports = ctx.accounts.destination.lamports();
-
-        **ctx.accounts.destination.lamports.borrow_mut() = dest_starting_lamports
-            .checked_add(account.lamports())
-            .unwrap();
-        **account.lamports.borrow_mut() = 0;
-
-        Ok(())
-    }
-
-...
-
-#[derive(Accounts)]
-pub struct ForceDefund<'info> {
-    account: AccountInfo<'info>,
-    destination: AccountInfo<'info>,
-}
-```
-
-Since anyone can call this instruction, this can act as a deterrent to attempted
-revival attacks since the attacker is paying for account rent exemption but
-anyone else can claim the lamports in a refunded account for themselves.
-
-While not necessary, this can help eliminate the waste of space and lamports
-associated with these "limbo" accounts.
-
 ### Use the Anchor `close` constraint
 
 Fortunately, Anchor makes all of this much simpler with the
@@ -240,7 +153,8 @@ everything required to securely close an account:
 
 1. Transfers the account’s lamports to the given `<target_account>`
 2. Zeroes out the account data
-3. Sets the account discriminator to the `CLOSED_ACCOUNT_DISCRIMINATOR` variant
+3. Assigning the owner of the account to the System Program and rellocating the
+   size of the account with 0 bytes.
 
 All you have to do is add it in the account validation struct to the account you
 want closed:
@@ -258,9 +172,6 @@ pub struct CloseAccount {
 }
 ```
 
-The `force_defund` instruction is an optional addition that you’ll have to
-implement on your own if you’d like to utilize it.
-
 ## Lab
 
 To clarify how an attacker might take advantage of a revival attack, let's work
@@ -270,7 +181,7 @@ participation in the lottery.
 ### 1. Setup
 
 Start by getting the code on the `starter` branch from the
-[following repo](https://github.com/Unboxed-Software/solana-closing-accounts/tree/starter).
+[following repo](https://github.com/solana-developers/closing-accounts/tree/starter).
 
 The code has two instructions on the program and two tests in the `tests`
 directory.
@@ -316,22 +227,27 @@ alive even after claiming rewards and then claim rewards again. That test looks
 like this:
 
 ```typescript
-it("attacker  can close + refund lottery acct + claim multiple rewards", async () => {
+it("attacker can close + refund lottery acct + claim multiple rewards successfully", async () => {
+  const [attackerLotteryEntry, bump] = PublicKey.findProgramAddressSync(
+    [Buffer.from("test-seed"), authority.publicKey.toBuffer()],
+    program.programId,
+  );
   // claim multiple times
   for (let i = 0; i < 2; i++) {
+    let tokenAcct = await getAccount(provider.connection, attackerAta);
+
     const tx = new Transaction();
+
     // instruction claims rewards, program will try to close account
     tx.add(
       await program.methods
         .redeemWinningsInsecure()
         .accounts({
-          lotteryEntry: attackerLotteryEntry,
-          user: attacker.publicKey,
           userAta: attackerAta,
           rewardMint: rewardMint,
-          mintAuth: mintAuth,
-          tokenProgram: TOKEN_PROGRAM_ID,
+          user: authority.publicKey,
         })
+        .signers([authority])
         .instruction(),
     );
 
@@ -343,21 +259,22 @@ it("attacker  can close + refund lottery acct + claim multiple rewards", async (
       );
     tx.add(
       SystemProgram.transfer({
-        fromPubkey: attacker.publicKey,
+        fromPubkey: authority.publicKey,
         toPubkey: attackerLotteryEntry,
         lamports: rentExemptLamports,
       }),
     );
     // send tx
-    await sendAndConfirmTransaction(provider.connection, tx, [attacker]);
+    await sendAndConfirmTransaction(provider.connection, tx, [authority]);
     await new Promise(x => setTimeout(x, 5000));
   }
 
-  const ata = await getAccount(provider.connection, attackerAta);
+  const tokenAcct = await getAccount(provider.connection, attackerAta);
+
   const lotteryEntry =
     await program.account.lotteryAccount.fetch(attackerLotteryEntry);
 
-  expect(Number(ata.amount)).to.equal(
+  expect(Number(tokenAcct.amount)).to.equal(
     lotteryEntry.timestamp.toNumber() * 10 * 2,
   );
 });
@@ -390,7 +307,7 @@ pub struct RedeemWinningsSecure<'info> {
     // program expects this account to be initialized
     #[account(
         mut,
-        seeds = [user.key().as_ref()],
+        seeds = [DATA_PDA_SEED.as_bytes(),user.key.as_ref()],
         bump = lottery_entry.bump,
         has_one = user,
         close = user
@@ -533,24 +450,27 @@ like this:
 
 ```bash
   closing-accounts
-    ✔ Enter lottery (451ms)
-    ✔ attacker can close + refund lottery acct + claim multiple rewards (18760ms)
-AnchorError caused by account: lottery_entry. Error Code: AccountDiscriminatorMismatch. Error Number: 3002. Error Message: 8 byte discriminator did not match what was expected.
-    ✔ attacker cannot claim multiple rewards with secure claim (414ms)
+    ✔ Enter lottery should be successful (451ms)
+    ✔ attacker can close + refund lottery acct + claim multiple rewards successfully (18760ms)
+AnchorError caused by account: lottery_entry. Error Code: AccountOwnedByWrongProgram. Error Number: 3007. Error Message: The given account is owned by a different program than expected.
+Program log: Left:
+Program log: 11111111111111111111111111111111
+Program log: Right:
+Program log: FqETzdh6PsE7aNjrdapuoyFeYGdjPKN8AgG2ZUghje8A
+    ✔ attacker cannot claim multiple rewards with secure claim successfully (414ms)
 ```
 
 Note, this does not prevent the malicious user from refunding their account
 altogether - it just protects our program from accidentally re-using the account
-when it should be closed. We haven't implemented a `force_defund` instruction so
-far, but we could. If you're feeling up for it, give it a try yourself!
+when it should be closed.
 
 The simplest and most secure way to close accounts is using Anchor's `close`
 constraint. If you ever need more custom behavior and can't use this constraint,
 make sure to replicate its functionality to ensure your program is secure.
 
 If you want to take a look at the final solution code you can find it on the
 `solution` branch of
-[the same repository](https://github.com/Unboxed-Software/solana-closing-accounts/tree/solution).
+[the same repository](https://github.com/solana-developers/closing-accounts/tree/solution).
 
 ## Challenge