NSSC Solana updates

Author: smoelius

Diff for: not-so-smart-contracts/solana/arbitrary_cpi/README.md

@@ -1,10 +1,10 @@
 # Arbitrary CPI
-Solana allows programs to call one another through cross-program invocation (CPI). This can be done via `invoke`, which is responsible for routing the passed in instruction to the program. Whenever an external contract is invoked via CPI, the program must check and verify the program ID. If the program ID isn't verified, then the contract can be called into an attacker-controlled contract instead of the intended one.
+Solana allows programs to call one another through cross-program invocation (CPI). This can be done via `invoke`, which is responsible for routing the passed in instruction to the program. Whenever an external contract is invoked via CPI, the program must check and verify the program ID. If the program ID isn't verified, then the contract can call an attacker-controlled program instead of the intended one.
 
 View ToB's lint implementation for the arbitrary CPI issue [here](https://github.com/crytic/solana-lints/tree/master/lints/arbitrary_cpi).
 
 ## Exploit Scenario
-Consider the following `withdraw` function. Tokens are able to be withdrawn from the pool to a user account. The program invoked here is user-controlled and there's no check that program passed in is the intended `token_program`. This allows a malicious user to pass in their own program with functionality to their discretion - such as draining the pool of the inputted `amount` tokens. 
+Consider the following `withdraw` function. Tokens are able to be withdrawn from the pool to a user account. The program invoked here is user-controlled and there's no check that the program passed in is the intended `token_program`. This allows a malicious user to pass in their own program with functionality to their discretion - such as draining the pool of the inputted `amount` tokens.
 ### Example Contract
 ```rust
    pub fn withdraw(accounts: &[AccountInfo], amount: u64) -> ProgramResult {
@@ -35,5 +35,5 @@ Consider the following `withdraw` function. Tokens are able to be withdrawn from
 ```rust
         if INPUTTED_PROGRAM.key != &INTENDED_PROGRAM::id() {
             return Err(ProgramError::InvalidProgramId);
-        }   
+        }
 ```

Diff for: not-so-smart-contracts/solana/improper_pda_validation/README.md

@@ -1,34 +1,31 @@
 
 # Improper PDA bump seed validation
 
-PDAs (Program Derived Addresses) are, by definition, [program-controlled](https://docs.solana.com/terminology#program-derived-account-pda) accounts and therefore can be used to sign without the need to provide a private key. PDAs are generated through a set of seeds and a program id, which are then collectively hashed to verify that the key doesn't lie on the ed25519 curve (curve used by Solana to sign transactions). 
+PDAs (Program Derived Addresses) are, by definition, [program-controlled](https://docs.solana.com/terminology#program-derived-account-pda) accounts and therefore can be used to sign without the need to provide a private key. PDAs are generated through a set of seeds and a program id, which are then collectively hashed to verify that the point doesn't lie on the ed25519 curve (the curve used by Solana to sign transactions).
 
-Values on this elliptic curve have a corresponding private key, which wouldn't make it a PDA. In the case a public key lying on the elliptic curve is found, our 32-byte address is modified through the addition of a bump to "bump" it off the curve. A bump, represented by a singular byte iterating through 255 to 0 is added onto our input until an address that doesn’t lie on the elliptic curve is generated, meaning that we’ve found an address without an associated private key. 
+Values on this elliptic curve have a corresponding private key, which wouldn't make it a PDA. In the case a point lying on the elliptic curve is found, our 32-byte address is modified through the addition of a bump to "bump" it off the curve. A bump, represented by a singular byte iterating through 255 to 0, is added onto our input until a point that doesn’t lie on the elliptic curve is generated, meaning that we’ve found an address without an associated private key.
 
-The issue arises with seeds being able to have multiple bumps, thus allowing varying PDAs that are valid from the same seed. An attacker can create a PDA with fabricated data - the program ID and seeds are the same as for the expected PDA but with different bump seeds. Without any explicit check against the bump seed itself, the program leaves itself vulnerable to the attacker tricking the program into thinking they’re the expected PDA and thus interacting with the contract on behalf of them.
+The issue arises with seeds being able to have multiple bumps, thus allowing varying PDAs that are valid from the same seeds. An attacker can create a PDA with the correct program ID but with a different bump. Without any explicit check against the bump seed itself, the program leaves itself vulnerable to the attacker tricking the program into thinking they’re using the expected PDA when in fact they're interacting with an illegitimate account.
 
 View ToB's lint implementation for the bump seed canonicalization issue [here](https://github.com/crytic/solana-lints/tree/master/lints/bump_seed_canonicalization).
 
 ## Exploit Scenario
 
-In Solana, the `create_program_address` function creates a 32-byte address based off the set of seeds and program address. On it's own, the address may lie on the ed25519 curve. Consider the following without any other validation being referenced within a sensitive function, such as one that handles transfers. That PDA could be spoofed by a passed in user-controlled PDA with malicious functions to their own discretion.  
+In Solana, the `create_program_address` function creates a 32-byte address based off the set of seeds and program address. On its own, the point may lie on the ed25519 curve. Consider the following without any other validation being referenced within a sensitive function, such as one that handles transfers. That PDA could be spoofed by a passed in user-controlled PDA.
 ### Example Contract
-```rust 
+```rust
 let program_address = Pubkey::create_program_address(&[key.to_le_bytes().as_ref(), &[reserve_bump]], program_id)?;
 
 ...
 ```
 ## Mitigation
 
-The `find_program_address` function finds the largest bump seeds for which there exists a corresponding PDA (i.e., an address not on the ed25519 curve), and returns both the address and the bump seed. The function panics in the case that no PDA address can be found.
+The `find_program_address` function finds the largest bump seeds for which there exists a corresponding PDA (i.e., a point not on the ed25519 curve), and returns both the address and the bump seed. The function panics in the case that no PDA address can be found.
 
 ```rust
-        let (address, system_bump) = Pubkey::find_program_address(&[key.to_le_bytes().as_ref()], program_id);
+        let (address, _system_bump) = Pubkey::find_program_address(&[key.to_le_bytes().as_ref()], program_id);
 
         if program_address != &account_data.key() {
             return Err(ProgramError::InvalidAddress);
         }
-        if system_bump != reserve_bump {
-            return Err(ProgramError::InvalidBump);
-        }
 ```

Diff for: not-so-smart-contracts/solana/ownership_check/README.md

@@ -1,35 +1,34 @@
 # Missing Ownership Check
-Accounts in Solana include metadata of an owner. These owners are identified by their own program ID. Without sufficient checks that the expected program ID matches that of the passed in account, an attacker can fabricate account data to pass any other preconditions and bypass the ownership check. 
+Accounts in Solana include metadata of an owner. These owners are identified by their own program ID. Without sufficient checks that the expected program ID matches that of the passed in account, an attacker can fabricate an account with spoofed data to pass any other preconditions.
 
-This malicious account will inherently have a different program ID, but considering there’s no check that the program ID is the same, as long as the other preconditions are passed the attacker can trick the program into thinking their malicious account is the expected authorized account.
+This malicious account will inherently have a different program ID as owner, but considering there’s no check that the program ID is the same, as long as the other preconditions are passed, the attacker can trick the program into thinking their malicious account is the expected account.
 
 ## Exploit Scenario
-The following contract updates the current market owner with a new one. Unfortunately, the only check being done here is against the current owner’s public key prior to setting a new owner. 
-Therefore, a malicious actor can set themselves as the new owner without being the actual market owner. This is because the ownership of the market owner account isn’t being fully verified against itself by program ID. Since there’s no check that the market is the one owned by the program itself, an attacker can pass in their own fabricated account data which is then verified against a public key of the current owner’s account, making it easy to set themselves as the new owner. 
+The following contract updates the current market authority with a new one. Unfortunately, the only check being done here is against the current authority’s public key prior to setting a new authority.
+Therefore, a malicious actor can set themselves as the new authority without being the actual market authority. This is because the ownership of the market authority account isn’t being fully verified against itself by program ID. Since there’s no check that the market is owned by the program itself, an attacker can pass in their own fabricated account with spoofed data which is then verified against the public key of the current authority’s account, making it easy for the attacker to set themselves as the new authority.
 
-### Example Contract 
+### Example Contract
 ```rust
-fn set_owner(program_id: &Pubkey, new_owner: Pubkey, accounts: &[AccountInfo]) -> ProgramResult {
+fn set_authority(program_id: &Pubkey, new_authority: Pubkey, accounts: &[AccountInfo]) -> ProgramResult {
 	let account_info_iter = &mut accounts.iter();
 	let market_info = next_account_info(account_info_iter)?;
-	let current_owner = next_account_info(account_info_iter)?;
+	let current_authority = next_account_info(account_info_iter)?;
 
 	let mut market = Market::unpack(&market_info.data.borrow())?;
- 
-	if &market.owner != current_owner.pubkey {
-    	    return Err(InvalidMarketOwner.into());
+
+	if &market.authority != current_authority.pubkey {
+    	    return Err(InvalidMarketAuthority.into());
 	}
-	market.owner = new_owner;
+	market.authority = new_authority;
 
   ...
- 
-	Ok(())
 
+	Ok(())
 }
 ```
 *Inspired by [SPL Lending Program](https://github.com/solana-labs/solana-program-library/tree/master/token-lending/program)*
 
-## Mitigation  
+## Mitigation
 
 ```rust
   	if EXPECTED_ACCOUNT.owner != program_id {

Diff for: not-so-smart-contracts/solana/signer_check/README.md

@@ -1,31 +1,29 @@
 # Missing Signer Check
-In Solana, a [transaction](https://docs.solana.com/developing/programming-model/transactions)  lists all account public keys used by the instructions of the transaction. These public keys come with a signature, ensuring that the account holder has authorized the transaction.
+In Solana, each public key has an associated private key that can be used to generate signatures. A [transaction](https://docs.solana.com/developing/programming-model/transactions) lists each account public key whose private key was used to generate a signature for the transaction. These signatures are verified using the inputted public keys prior to transaction execution.
 
-Specifically, these signatures used to validate transactions are generated by the account’s private key and are verified by being matched against an inputted public key prior to successful transaction execution.
-In case certain permissions are required to perform a sensitive function of the contract, a missing signer check becomes an issue. Any user’s account with the authorized account’s public key will be able to call the respective access controlled functions permissionlessly without ever having to validate their private key.
+In case certain permissions are required to perform a sensitive function of the contract, a missing signer check becomes an issue. Without this check, an attacker would be able to call the respective access controlled functions permissionlessly.
 
 ## Exploit Scenario
-The following contract updates the current market owner with a new one. Unfortunately, the only check being done here is against the current owner’s public key prior to setting a new owner. 
-Therefore, a malicious actor can set themselves as the new owner without being the actual market owner. This is because the current owner’s private key isn’t being verified considering the contract doesn’t check whether the account holder has signed or not.
+The following contract updates the current market authority with a new one. Unfortunately, the only check being done here is against the current authority’s public key prior to setting a new authority.
+Therefore, a malicious actor can set themselves as the new authority without being the actual market authority. This is because the current authority’s private key isn’t being verified considering the contract doesn’t check whether the account holder has signed or not.
 
-### Example Contract 
+### Example Contract
 ```rust
-fn set_owner(program_id: &Pubkey, new_owner: Pubkey, accounts: &[AccountInfo]) -> ProgramResult {
+fn set_authority(program_id: &Pubkey, new_authority: Pubkey, accounts: &[AccountInfo]) -> ProgramResult {
 	let account_info_iter = &mut accounts.iter();
 	let market_info = next_account_info(account_info_iter)?;
-	let current_owner = next_account_info(account_info_iter)?;
+	let current_authority = next_account_info(account_info_iter)?;
 
 	let mut market = Market::unpack(&market_info.data.borrow())?;
- 
-	if &market.owner != current_owner.pubkey {
-    	    return Err(InvalidMarketOwner.into());
+
+	if &market.authority != current_authority.pubkey {
+    	    return Err(InvalidMarketAuthority.into());
 	}
-	market.owner = new_owner;
+	market.authority = new_authority;
 
   ...
- 
-	Ok(())
 
+	Ok(())
 }
 ```
 *Inspired by [SPL Lending Program](https://github.com/solana-labs/solana-program-library/tree/master/token-lending/program)*

Diff for: not-so-smart-contracts/solana/sysvar_account_check/README.md

@@ -3,11 +3,11 @@ The sysvar (system account) account is often used while validating access contro
 
 ## Exploit Scenario
 
-secp256k1 is an elliptic curve used by a number of blockchains for signatures. Validating signatures is crucial as by bypassing signature checks, an attacker can gain access to restricted functions that could lead to drainage of funds. 
+secp256k1 is an elliptic curve used by a number of blockchains for signatures. Validating signatures is crucial as by bypassing signature checks, an attacker can gain access to restricted functions that could lead to drainage of funds.
 
-Here, `load_current_index` and `load_instruction_at` are functions that don't verify that the inputted sysvar account is authorized, therefore allowing serialized maliciously fabricated data to sucessfully spoof as an authorized secp256k1 signature. 
+Here, `load_current_index` and `load_instruction_at` are functions that don't verify that the inputted sysvar account is authorized, therefore allowing serialized maliciously fabricated data to sucessfully spoof as an authorized secp256k1 signature.
 
-### Example Contract 
+### Example Contract
 ```rust
 pub fn verify_signatures(account_info: &AccountInfo) -> ProgramResult {
     let index = solana_program::sysvar::instructions::load_current_index(
@@ -30,7 +30,7 @@ Refer to [Mitigation](https://github.com/crytic/building-secure-contracts/tree/m
 - Solana libraries should be running on version 1.8.1 and up
 - Use `load_instruction_at_checked` and `load_current_index_checked`
 
-Utilizing the latest Solana version and referencing checked functions, especially on sensitive parts of a contract is crucial even if potential attack vectors have been fixed post-audit. 
+Utilizing the latest Solana version and referencing checked functions, especially on sensitive parts of a contract is crucial even if potential attack vectors have been fixed post-audit.
 Leaving the system exposed to any point of failure compromises the entire system's integrity, especially while the contracts are being constantly updated.
 
 Here is the code showing the sysvar account checks added between unchecked and checked functions:
@@ -43,7 +43,7 @@ Here is the code showing the sysvar account checks added between unchecked and c
 
 **Note: The following analysis is condensed down to be present this attack vector as clearly as possible, and certain details might’ve been left out for the sake of simplification**
 
-The Wormhole hack serves to be one of the most memorable exploits in terms of impact DeFi has ever seen. 
+The Wormhole hack serves to be one of the most memorable exploits in terms of impact DeFi has ever seen.
 
 This exploit also happens to incorporate a missing sysvar account check that allowed the attacker to:
 1. Spoof Guardian signatures as valid