Track account writable deescalation

solana-labs · Jan 18, 2021 · 8a14638 · 8a14638
1 parent d63b2ba
commit 8a14638
Show file tree

Hide file tree

Showing 9 changed files with 105 additions and 47 deletions.
diff --git a/programs/bpf/c/src/invoke/invoke.c b/programs/bpf/c/src/invoke/invoke.c
@@ -15,6 +15,7 @@ static const uint8_t TEST_ALLOC_ACCESS_VIOLATION = 8;
 static const uint8_t TEST_INSTRUCTION_DATA_TOO_LARGE = 9;
 static const uint8_t TEST_INSTRUCTION_META_TOO_LARGE = 10;
 static const uint8_t TEST_RETURN_ERROR = 11;
+static const uint8_t TEST_WRITE_DEESCALATION = 12;
 
 static const int MINT_INDEX = 0;
 static const int ARGUMENT_INDEX = 1;
@@ -427,7 +428,8 @@ extern uint64_t entrypoint(const uint8_t *input) {
     break;
   }
   case TEST_RETURN_ERROR: {
-    SolAccountMeta arguments[] = {{accounts[ARGUMENT_INDEX].key, true, true}};
+    sol_log("Test return error");
+    SolAccountMeta arguments[] = {{accounts[ARGUMENT_INDEX].key, false, true}};
     uint8_t data[] = {RETURN_ERROR};
     const SolInstruction instruction = {accounts[INVOKED_PROGRAM_INDEX].key,
                                         arguments, SOL_ARRAY_SIZE(arguments),
@@ -436,6 +438,18 @@ extern uint64_t entrypoint(const uint8_t *input) {
     sol_invoke(&instruction, accounts, SOL_ARRAY_SIZE(accounts));
     break;
   }
+  case TEST_WRITE_DEESCALATION: {
+    sol_log("Test writable deescalation");
+
+    SolAccountMeta arguments[] = {
+        {accounts[INVOKED_ARGUMENT_INDEX].key, false, false}};
+    uint8_t data[] = {WRITE_ACCOUNT, 10};
+    const SolInstruction instruction = {accounts[INVOKED_PROGRAM_INDEX].key,
+                                        arguments, SOL_ARRAY_SIZE(arguments),
+                                        data, SOL_ARRAY_SIZE(data)};
+    sol_invoke(&instruction, accounts, SOL_ARRAY_SIZE(accounts));
+    break;
+  }
   default:
     sol_panic();
   }

diff --git a/programs/bpf/c/src/invoked/instruction.h b/programs/bpf/c/src/invoked/instruction.h
@@ -12,3 +12,4 @@ const uint8_t VERIFY_WRITER = 4;
 const uint8_t VERIFY_PRIVILEGE_ESCALATION = 5;
 const uint8_t NESTED_INVOKE = 6;
 const uint8_t RETURN_OK = 7;
+const uint8_t WRITE_ACCOUNT = 8;
diff --git a/programs/bpf/c/src/invoked/invoked.c b/programs/bpf/c/src/invoked/invoked.c
@@ -156,10 +156,12 @@ extern uint64_t entrypoint(const uint8_t *input) {
     sol_assert(accounts[ARGUMENT_INDEX].is_writable);
     break;
   }
+
   case VERIFY_PRIVILEGE_ESCALATION: {
     sol_log("Success");
     break;
   }
+
   case NESTED_INVOKE: {
     sol_log("invoke");
 
@@ -198,6 +200,18 @@ extern uint64_t entrypoint(const uint8_t *input) {
     }
     break;
   }
+
+  case WRITE_ACCOUNT: {
+    sol_log("write account");
+    static const int INVOKED_ARGUMENT_INDEX = 0;
+    sol_assert(sol_deserialize(input, &params, 1));
+
+    for (int i = 0; i < params.data[1]; i++) {
+      accounts[INVOKED_ARGUMENT_INDEX].data[i] = params.data[1];
+    }
+    break;
+  }
+
   default:
     return ERROR_INVALID_INSTRUCTION_DATA;
   }

diff --git a/programs/bpf/rust/invoke/src/lib.rs b/programs/bpf/rust/invoke/src/lib.rs
@@ -27,6 +27,7 @@ const TEST_ALLOC_ACCESS_VIOLATION: u8 = 8;
 const TEST_INSTRUCTION_DATA_TOO_LARGE: u8 = 9;
 const TEST_INSTRUCTION_META_TOO_LARGE: u8 = 10;
 const TEST_RETURN_ERROR: u8 = 11;
+const TEST_WRITE_DEESCALATION: u8 = 12;
 
 // const MINT_INDEX: usize = 0;
 const ARGUMENT_INDEX: usize = 1;
@@ -492,6 +493,15 @@ fn process_instruction(
             );
             let _ = invoke(&instruction, accounts);
         }
+        TEST_WRITE_DEESCALATION => {
+            msg!("Test writable deescalation");
+            let instruction = create_instruction(
+                *accounts[INVOKED_PROGRAM_INDEX].key,
+                &[(accounts[INVOKED_ARGUMENT_INDEX].key, false, false)],
+                vec![WRITE_ACCOUNT, 10],
+            );
+            let _ = invoke(&instruction, accounts);
+        }
         _ => panic!(),
     }
 

diff --git a/programs/bpf/rust/invoked/src/instruction.rs b/programs/bpf/rust/invoked/src/instruction.rs
@@ -13,6 +13,7 @@ pub const VERIFY_WRITER: u8 = 4;
 pub const VERIFY_PRIVILEGE_ESCALATION: u8 = 5;
 pub const NESTED_INVOKE: u8 = 6;
 pub const RETURN_OK: u8 = 7;
+pub const WRITE_ACCOUNT: u8 = 8;
 
 pub fn create_instruction(
     program_id: Pubkey,

diff --git a/programs/bpf/rust/invoked/src/processor.rs b/programs/bpf/rust/invoked/src/processor.rs
@@ -195,6 +195,12 @@ fn process_instruction(
                 }
             }
         }
+        WRITE_ACCOUNT => {
+            msg!("write account");
+            for i in 0..instruction_data[1] {
+                accounts[0].data.borrow_mut()[i as usize] = instruction_data[1];
+            }
+        }
         _ => panic!(),
     }
 

diff --git a/programs/bpf/tests/programs.rs b/programs/bpf/tests/programs.rs
@@ -556,7 +556,7 @@ fn test_program_bpf_error_handling() {
 }
 
 #[test]
-fn test_program_bpf_invoke() {
+fn test_program_bpf_invoke_sanity() {
     solana_logger::setup();
 
     const TEST_SUCCESS: u8 = 1;
@@ -570,6 +570,7 @@ fn test_program_bpf_invoke() {
     const TEST_INSTRUCTION_DATA_TOO_LARGE: u8 = 9;
     const TEST_INSTRUCTION_META_TOO_LARGE: u8 = 10;
     const TEST_RETURN_ERROR: u8 = 11;
+    const TEST_WRITE_DEESCALATION: u8 = 12;
 
     #[allow(dead_code)]
     #[derive(Debug)]
@@ -988,6 +989,33 @@ fn test_program_bpf_invoke() {
             TransactionError::InstructionError(0, InstructionError::Custom(42))
         );
 
+        let instruction = Instruction::new(
+            invoke_program_id,
+            &[TEST_WRITE_DEESCALATION, bump_seed1, bump_seed2, bump_seed3],
+            account_metas.clone(),
+        );
+        let message = Message::new(&[instruction], Some(&mint_pubkey));
+        let tx = Transaction::new(
+            &[
+                &mint_keypair,
+                &argument_keypair,
+                &invoked_argument_keypair,
+                &from_keypair,
+            ],
+            message.clone(),
+            bank.last_blockhash(),
+        );
+        let (result, inner_instructions) = process_transaction_and_record_inner(&bank, tx);
+        let invoked_programs: Vec<Pubkey> = inner_instructions[0]
+            .iter()
+            .map(|ix| message.account_keys[ix.program_id_index as usize].clone())
+            .collect();
+        assert_eq!(invoked_programs, vec![invoked_program_id.clone()]);
+        assert_eq!(
+            result.unwrap_err(),
+            TransactionError::InstructionError(0, InstructionError::ReadonlyDataModified)
+        );
+
         // Check final state
 
         assert_eq!(43, bank.get_balance(&derived_key1));

diff --git a/runtime/benches/message_processor.rs b/runtime/benches/message_processor.rs
@@ -13,18 +13,13 @@ fn bench_verify_account_changes_data(bencher: &mut Bencher) {
 
     let owner = pubkey::new_rand();
     let non_owner = pubkey::new_rand();
-    let pre = PreAccount::new(
-        &pubkey::new_rand(),
-        &Account::new(0, BUFSIZE, &owner),
-        true,
-        false,
-    );
+    let pre = PreAccount::new(&pubkey::new_rand(), &Account::new(0, BUFSIZE, &owner));
     let post = Account::new(0, BUFSIZE, &owner);
-    assert_eq!(pre.verify(&owner, &Rent::default(), &post), Ok(()));
+    assert_eq!(pre.verify(&owner, false, &Rent::default(), &post), Ok(()));
 
     // this one should be faster
     bencher.iter(|| {
-        pre.verify(&owner, &Rent::default(), &post).unwrap();
+        pre.verify(&owner, false, &Rent::default(), &post).unwrap();
     });
     let summary = bencher.bench(|_bencher| {}).unwrap();
     info!("data no change by owner: {} ns/iter", summary.median);
@@ -35,14 +30,10 @@ fn bench_verify_account_changes_data(bencher: &mut Bencher) {
     let summary = bencher.bench(|_bencher| {}).unwrap();
     info!("data compare {} ns/iter", summary.median);
 
-    let pre = PreAccount::new(
-        &pubkey::new_rand(),
-        &Account::new(0, BUFSIZE, &owner),
-        true,
-        false,
-    );
+    let pre = PreAccount::new(&pubkey::new_rand(), &Account::new(0, BUFSIZE, &owner));
     bencher.iter(|| {
-        pre.verify(&non_owner, &Rent::default(), &post).unwrap();
+        pre.verify(&non_owner, false, &Rent::default(), &post)
+            .unwrap();
     });
     let summary = bencher.bench(|_bencher| {}).unwrap();
     info!("data no change by non owner: {} ns/iter", summary.median);