From bad0b55ab66a9230d7a39a5b4d24546dc7d19088 Mon Sep 17 00:00:00 2001 From: Michael Vines Date: Thu, 29 Nov 2018 12:32:16 -0800 Subject: [PATCH] Expose which keys signed the Transaction in the SDK --- programs/bpf/c/sdk/inc/solana_sdk.h | 14 +++++--- programs/bpf/c/src/move_funds.c | 5 +++ programs/native/bpf_loader/src/lib.rs | 14 ++++++-- programs/native/erc20/src/token_program.rs | 25 ++++++++------- programs/native/lua_loader/src/lib.rs | 18 +++++++++-- sdk/src/account.rs | 37 ++++++++++++++++++++-- sdk/src/transaction.rs | 14 ++++---- src/budget_program.rs | 4 +-- src/native_loader.rs | 9 +++++- src/runtime.rs | 9 ++++-- src/storage_program.rs | 2 +- src/system_program.rs | 2 +- src/vote_program.rs | 2 +- 13 files changed, 116 insertions(+), 39 deletions(-) diff --git a/programs/bpf/c/sdk/inc/solana_sdk.h b/programs/bpf/c/sdk/inc/solana_sdk.h index 57053a7fe8..31f33a0387 100644 --- a/programs/bpf/c/sdk/inc/solana_sdk.h +++ b/programs/bpf/c/sdk/inc/solana_sdk.h @@ -110,11 +110,12 @@ SOL_FN_PREFIX bool SolPubkey_same(const SolPubkey *one, const SolPubkey *two) { * Keyed Account */ typedef struct { - SolPubkey *key; /** Public Key of the account */ - uint64_t *tokens; /** Numer of tokens owned by this account */ - uint64_t userdata_len; /** Length of userdata in bytes */ - uint8_t *userdata; /** On-chain data owned by this account */ - SolPubkey *owner; /** Program that owns this account */ + SolPubkey *key; /** Public key of the account */ + bool is_signer; /** Transaction was signed by this account's key */ + uint64_t *tokens; /** Number of tokens owned by this account */ + uint64_t userdata_len; /** Length of data in bytes */ + uint8_t *userdata; /** On-chain data within this account */ + SolPubkey *owner; /** Program that owns this account */ } SolKeyedAccount; /** @@ -243,6 +244,8 @@ SOL_FN_PREFIX bool sol_deserialize( input += sizeof(uint64_t); for (int i = 0; i < ka_len; i++) { // key + ka[i].is_signer = *(uint64_t *) input != 0; + input += sizeof(uint64_t); ka[i].key = (SolPubkey *) input; input += sizeof(SolPubkey); @@ -319,6 +322,7 @@ SOL_FN_PREFIX void sol_log_params( ) { sol_log_64(0, 0, 0, 0, ka_len); for (int i = 0; i < ka_len; i++) { + sol_log_64(0, 0, 0, 0, ka[i].is_signer); sol_log_key(ka[i].key); sol_log_64(0, 0, 0, 0, *ka[i].tokens); sol_log_array(ka[i].userdata, ka[i].userdata_len); diff --git a/programs/bpf/c/src/move_funds.c b/programs/bpf/c/src/move_funds.c index 6a7ae18467..514d397c09 100644 --- a/programs/bpf/c/src/move_funds.c +++ b/programs/bpf/c/src/move_funds.c @@ -20,6 +20,11 @@ extern bool entrypoint(const uint8_t *input) { return false; } + if (!ka[0].is_signer) { + sol_log("Transaction not signed by key 0"); + return false; + } + int64_t tokens = *(int64_t *)data; if (*ka[0].tokens >= tokens) { *ka[0].tokens -= tokens; diff --git a/programs/native/bpf_loader/src/lib.rs b/programs/native/bpf_loader/src/lib.rs index 1686446a3e..1e49a83106 100644 --- a/programs/native/bpf_loader/src/lib.rs +++ b/programs/native/bpf_loader/src/lib.rs @@ -110,7 +110,9 @@ fn serialize_parameters( v.write_u64::(keyed_accounts.len() as u64) .unwrap(); for info in keyed_accounts.iter_mut() { - v.write_all(info.key.as_ref()).unwrap(); + v.write_u64::(info.signer_key().is_some() as u64) + .unwrap(); + v.write_all(info.unsigned_key().as_ref()).unwrap(); v.write_u64::(info.account.tokens).unwrap(); v.write_u64::(info.account.userdata.len() as u64) .unwrap(); @@ -129,6 +131,7 @@ fn deserialize_parameters(keyed_accounts: &mut [KeyedAccount], buffer: &[u8]) { let mut start = mem::size_of::(); for info in keyed_accounts.iter_mut() { + start += mem::size_of::(); // skip signer_key boolean start += mem::size_of::(); // skip pubkey info.account.tokens = LittleEndian::read_u64(&buffer[start..]); @@ -185,6 +188,10 @@ fn entrypoint( vm.get_last_instruction_count() ); } else if let Ok(instruction) = deserialize(tx_data) { + if keyed_accounts[0].signer_key().is_none() { + warn!("key[0] did not sign the transaction"); + return false; + } match instruction { LoaderInstruction::Write { offset, bytes } => { let offset = offset as usize; @@ -202,7 +209,10 @@ fn entrypoint( } LoaderInstruction::Finalize => { keyed_accounts[0].account.executable = true; - info!("Finalize: account {:?}", keyed_accounts[0].key); + info!( + "Finalize: account {:?}", + keyed_accounts[0].signer_key().unwrap() + ); } } } else { diff --git a/programs/native/erc20/src/token_program.rs b/programs/native/erc20/src/token_program.rs index bda87159b4..b077199993 100644 --- a/programs/native/erc20/src/token_program.rs +++ b/programs/native/erc20/src/token_program.rs @@ -182,7 +182,7 @@ impl TokenProgram { } if let TokenProgram::Account(dest_account) = &input_program_accounts[1] { - if info[0].key != &dest_account.token { + if info[0].signer_key().unwrap() != &dest_account.token { error!("account 1 token mismatch"); Err(Error::InvalidArgument)?; } @@ -225,16 +225,15 @@ impl TokenProgram { error!("account 0 is already allocated"); Err(Error::InvalidArgument)?; } - let mut token_account_info = TokenAccountInfo { - token: *info[2].key, - owner: *info[1].key, + token: *info[2].unsigned_key(), + owner: *info[1].unsigned_key(), amount: 0, delegate: None, }; if input_program_accounts.len() >= 4 { token_account_info.delegate = Some(TokenAccountDelegateInfo { - source: *info[3].key, + source: *info[3].unsigned_key(), original_amount: 0, }); } @@ -266,7 +265,7 @@ impl TokenProgram { Err(Error::InvalidArgument)?; } - if info[0].key != &source_account.owner { + if info[0].signer_key().unwrap() != &source_account.owner { error!("owner of account 1 not present"); Err(Error::InvalidArgument)?; } @@ -291,7 +290,7 @@ impl TokenProgram { error!("account 1/3 token mismatch"); Err(Error::InvalidArgument)?; } - if info[3].key != &delegate_info.source { + if info[3].unsigned_key() != &delegate_info.source { error!("Account 1 is not a delegate of account 3"); Err(Error::InvalidArgument)?; } @@ -338,7 +337,7 @@ impl TokenProgram { Err(Error::InvalidArgument)?; } - if info[0].key != &source_account.owner { + if info[0].signer_key().unwrap() != &source_account.owner { error!("owner of account 1 not present"); Err(Error::InvalidArgument)?; } @@ -354,7 +353,7 @@ impl TokenProgram { Err(Error::InvalidArgument)?; } Some(delegate_info) => { - if info[1].key != &delegate_info.source { + if info[1].unsigned_key() != &delegate_info.source { error!("account 2 is not a delegate of account 1"); Err(Error::InvalidArgument)?; } @@ -387,13 +386,13 @@ impl TokenProgram { } if let TokenProgram::Account(source_account) = &input_program_accounts[1] { - if info[0].key != &source_account.owner { + if info[0].signer_key().unwrap() != &source_account.owner { info!("owner of account 1 not present"); Err(Error::InvalidArgument)?; } let mut output_source_account = source_account.clone(); - output_source_account.owner = *info[2].key; + output_source_account.owner = *info[2].unsigned_key(); output_program_accounts.push((1, TokenProgram::Account(output_source_account))); } else { info!("account 1 is invalid"); @@ -406,6 +405,10 @@ impl TokenProgram { let command = bincode::deserialize::(input).map_err(Self::map_to_invalid_args)?; info!("process_transaction: command={:?}", command); + if info[0].signer_key().is_none() { + Err(Error::InvalidArgument)?; + } + let input_program_accounts: Vec = info .iter() .map(|keyed_account| { diff --git a/programs/native/lua_loader/src/lib.rs b/programs/native/lua_loader/src/lib.rs index 636571ae03..bc031f6f42 100644 --- a/programs/native/lua_loader/src/lib.rs +++ b/programs/native/lua_loader/src/lib.rs @@ -19,7 +19,14 @@ fn set_accounts(lua: &Lua, name: &str, keyed_accounts: &[KeyedAccount]) -> Resul let accounts = lua.create_table()?; for (i, keyed_account) in keyed_accounts.iter().enumerate() { let account = lua.create_table()?; - account.set("key", keyed_account.key.to_string())?; + account.set( + if keyed_account.signer_key().is_some() { + "signer_key" + } else { + "unsigned_key" + }, + keyed_account.unsigned_key().to_string(), + )?; account.set("tokens", keyed_account.account.tokens)?; let data_str = lua.create_string(&keyed_account.account.userdata)?; account.set("userdata", data_str)?; @@ -80,6 +87,10 @@ fn entrypoint( } } } else if let Ok(instruction) = deserialize(tx_data) { + if keyed_accounts[0].signer_key().is_none() { + warn!("key[0] did not sign the transaction"); + return false; + } match instruction { LoaderInstruction::Write { offset, bytes } => { let offset = offset as usize; @@ -98,7 +109,10 @@ fn entrypoint( LoaderInstruction::Finalize => { keyed_accounts[0].account.executable = true; - trace!("LuaLoader::Finalize prog: {:?}", keyed_accounts[0].key); + trace!( + "LuaLoader::Finalize prog: {:?}", + keyed_accounts[0].signer_key().unwrap() + ); } } } else { diff --git a/sdk/src/account.rs b/sdk/src/account.rs index 901c7bf48f..c282f316a7 100644 --- a/sdk/src/account.rs +++ b/sdk/src/account.rs @@ -33,19 +33,50 @@ impl Account { #[repr(C)] #[derive(Debug)] pub struct KeyedAccount<'a> { - pub key: &'a Pubkey, + is_signer: bool, // Transaction was signed by this account's key + key: &'a Pubkey, pub account: &'a mut Account, } +impl<'a> KeyedAccount<'a> { + pub fn signer_key(&self) -> Option<&Pubkey> { + if self.is_signer { + Some(self.key) + } else { + None + } + } + + pub fn unsigned_key(&self) -> &Pubkey { + self.key + } + + pub fn new(key: &'a Pubkey, is_signer: bool, account: &'a mut Account) -> KeyedAccount<'a> { + KeyedAccount { + key, + is_signer, + account, + } + } +} + impl<'a> From<(&'a Pubkey, &'a mut Account)> for KeyedAccount<'a> { fn from((key, account): (&'a Pubkey, &'a mut Account)) -> Self { - KeyedAccount { key, account } + KeyedAccount { + is_signer: false, + key, + account, + } } } impl<'a> From<&'a mut (Pubkey, Account)> for KeyedAccount<'a> { fn from((key, account): &'a mut (Pubkey, Account)) -> Self { - KeyedAccount { key, account } + KeyedAccount { + is_signer: false, + key, + account, + } } } diff --git a/sdk/src/transaction.rs b/sdk/src/transaction.rs index cf59000f3a..cb4e174ac2 100644 --- a/sdk/src/transaction.rs +++ b/sdk/src/transaction.rs @@ -126,7 +126,7 @@ impl Transaction { self.key_index(instruction_index, accounts_index) .and_then(|account_keys_index| self.account_keys.get(account_keys_index)) } - pub fn signed_key(&self, instruction_index: usize, accounts_index: usize) -> Option<&Pubkey> { + pub fn signer_key(&self, instruction_index: usize, accounts_index: usize) -> Option<&Pubkey> { match self.key_index(instruction_index, accounts_index) { None => None, Some(signature_index) => { @@ -232,22 +232,22 @@ mod tests { assert!(tx.verify_refs()); assert_eq!(tx.key(0, 0), Some(&key.pubkey())); - assert_eq!(tx.signed_key(0, 0), Some(&key.pubkey())); + assert_eq!(tx.signer_key(0, 0), Some(&key.pubkey())); assert_eq!(tx.key(1, 0), Some(&key.pubkey())); - assert_eq!(tx.signed_key(1, 0), Some(&key.pubkey())); + assert_eq!(tx.signer_key(1, 0), Some(&key.pubkey())); assert_eq!(tx.key(0, 1), Some(&key1)); - assert_eq!(tx.signed_key(0, 1), None); + assert_eq!(tx.signer_key(0, 1), None); assert_eq!(tx.key(1, 1), Some(&key2)); - assert_eq!(tx.signed_key(1, 1), None); + assert_eq!(tx.signer_key(1, 1), None); assert_eq!(tx.key(2, 0), None); - assert_eq!(tx.signed_key(2, 0), None); + assert_eq!(tx.signer_key(2, 0), None); assert_eq!(tx.key(0, 2), None); - assert_eq!(tx.signed_key(0, 2), None); + assert_eq!(tx.signer_key(0, 2), None); assert_eq!(*tx.program_id(0), prog1); assert_eq!(*tx.program_id(1), prog2); diff --git a/src/budget_program.rs b/src/budget_program.rs index 975c84bbd8..499853a6ee 100644 --- a/src/budget_program.rs +++ b/src/budget_program.rs @@ -170,7 +170,7 @@ impl BudgetProgram { ) -> Result<(), BudgetError> { let mut final_payment = None; if let Some(ref mut expr) = self.pending_budget { - let key = match tx.signed_key(instruction_index, 0) { + let key = match tx.signer_key(instruction_index, 0) { None => return Err(BudgetError::UnsignedKey), Some(key) => key, }; @@ -203,7 +203,7 @@ impl BudgetProgram { let mut final_payment = None; if let Some(ref mut expr) = self.pending_budget { - let key = match tx.signed_key(instruction_index, 0) { + let key = match tx.signer_key(instruction_index, 0) { None => return Err(BudgetError::UnsignedKey), Some(key) => key, }; diff --git a/src/native_loader.rs b/src/native_loader.rs index 27885990e8..358f04f645 100644 --- a/src/native_loader.rs +++ b/src/native_loader.rs @@ -99,6 +99,10 @@ pub fn process_instruction( } } } else if let Ok(instruction) = deserialize(ix_userdata) { + if keyed_accounts[0].signer_key().is_none() { + warn!("key[0] did not sign the transaction"); + return false; + } match instruction { LoaderInstruction::Write { offset, bytes } => { trace!("NativeLoader::Write offset {} bytes {:?}", offset, bytes); @@ -117,7 +121,10 @@ pub fn process_instruction( LoaderInstruction::Finalize => { keyed_accounts[0].account.executable = true; - trace!("NativeLoader::Finalize prog: {:?}", keyed_accounts[0].key); + trace!( + "NativeLoader::Finalize prog: {:?}", + keyed_accounts[0].signer_key().unwrap() + ); } } } else { diff --git a/src/runtime.rs b/src/runtime.rs index 766a395b12..2cdd90e722 100644 --- a/src/runtime.rs +++ b/src/runtime.rs @@ -52,9 +52,12 @@ fn process_instruction( let mut keyed_accounts2: Vec<_> = tx.instructions[instruction_index] .accounts .iter() - .map(|&index| &tx.account_keys[index as usize]) - .zip(program_accounts.iter_mut()) - .map(|(key, account)| KeyedAccount { key, account }) + .map(|&index| { + let index = index as usize; + let key = &tx.account_keys[index]; + (key, index < tx.signatures.len()) + }).zip(program_accounts.iter_mut()) + .map(|((key, is_signer), account)| KeyedAccount::new(key, is_signer, account)) .collect(); keyed_accounts.append(&mut keyed_accounts2); diff --git a/src/storage_program.rs b/src/storage_program.rs index 7aa04d0577..11a0d7b8be 100644 --- a/src/storage_program.rs +++ b/src/storage_program.rs @@ -42,7 +42,7 @@ pub fn process_instruction( _accounts: &mut [&mut Account], ) -> Result<(), StorageError> { // accounts_keys[0] must be signed - if tx.signed_key(pix, 0).is_none() { + if tx.signer_key(pix, 0).is_none() { Err(StorageError::InvalidArgument)?; } diff --git a/src/system_program.rs b/src/system_program.rs index 2691864bde..afccaabc32 100644 --- a/src/system_program.rs +++ b/src/system_program.rs @@ -43,7 +43,7 @@ fn process_instruction(tx: &Transaction, pix: usize, accounts: &mut [&mut Accoun let from = 0; // all system instructions require that accounts_keys[0] be a signer - if tx.signed_key(pix, from).is_none() { + if tx.signer_key(pix, from).is_none() { Err(Error::InvalidArgument)?; } diff --git a/src/vote_program.rs b/src/vote_program.rs index 3e19d05b8e..77a0eb5720 100644 --- a/src/vote_program.rs +++ b/src/vote_program.rs @@ -71,7 +71,7 @@ pub fn process_instruction( accounts: &mut [&mut Account], ) -> Result<()> { // all vote instructions require that accounts_keys[0] be a signer - if tx.signed_key(instruction_index, 0).is_none() { + if tx.signer_key(instruction_index, 0).is_none() { Err(Error::InvalidArguments)?; }