gaspersic/solana
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Prevent privilege escalation (#10232)

Browse Source 
automerge
This commit is contained in:
Jack May
2020-05-26 01:02:31 -07:00
committed by GitHub
parent 5d96fcec63
commit 03abd3ddd7
13 changed files with 458 additions and 392 deletions
Download Patch File Download Diff File Expand all files Collapse all files

146
runtime/src/message_processor.rs
View File

@ -122,31 +122,6 @@ impl PreAccount {
Ok(())
}
pub fn verify_cross_program(
&self,
is_writable: bool,
is_signer: bool,
signers: &[Pubkey],
program_id: &Pubkey,
rent: &Rent,
post: &Account,
) -> Result<(), InstructionError> {
// Readonly account cannot become writable
if is_writable && !self.is_writable {
return Err(InstructionError::WritableModified);
}
if is_signer && // If message indicates account is signed
!( // one of the following needs to be true:
self.is_signer // Signed in the original transaction
|| signers.contains(&self.key) // Signed by the program
) {
return Err(InstructionError::SignerModified);
}
self.verify(program_id, rent, post)
}
pub fn update(&mut self, account: &Account) {
self.lamports = account.lamports;
if self.data.len() != account.data.len() {
@ -213,7 +188,6 @@ impl InvokeContext for ThisInvokeContext {
&mut self,
message: &Message,
instruction: &CompiledInstruction,
signers: &[Pubkey],
accounts: &[Rc<RefCell<Account>>],
) -> Result<(), InstructionError> {
match self.program_ids.last() {
@ -221,10 +195,9 @@ impl InvokeContext for ThisInvokeContext {
message,
instruction,
&mut self.pre_accounts,
accounts,
key,
&self.rent,
signers,
accounts,
),
None => Err(InstructionError::GenericError), // Should never happen
}
@ -353,13 +326,12 @@ impl MessageProcessor {
message: &Message,
executable_accounts: &[(Pubkey, RefCell<Account>)],
accounts: &[Rc<RefCell<Account>>],
signers: &[Pubkey],
invoke_context: &mut dyn InvokeContext,
) -> Result<(), InstructionError> {
let instruction = &message.instructions[0];
// Verify the calling program hasn't misbehaved
invoke_context.verify_and_update(message, instruction, signers, accounts)?;
invoke_context.verify_and_update(message, instruction, accounts)?;
// Construct keyed accounts
let keyed_accounts =
@ -371,7 +343,7 @@ impl MessageProcessor {
self.process_instruction(&keyed_accounts, &instruction.data, invoke_context);
if result.is_ok() {
// Verify the called program has not misbehaved
result = invoke_context.verify_and_update(message, instruction, signers, accounts);
result = invoke_context.verify_and_update(message, instruction, accounts);
}
invoke_context.pop();
@ -419,7 +391,7 @@ impl MessageProcessor {
pre_accounts: &[PreAccount],
executable_accounts: &[(Pubkey, RefCell<Account>)],
accounts: &[Rc<RefCell<Account>>],
rent_collector: &RentCollector,
rent: &Rent,
) -> Result<(), InstructionError> {
// Verify all executable accounts have zero outstanding refs
Self::verify_account_references(executable_accounts)?;
@ -433,7 +405,7 @@ impl MessageProcessor {
let account = accounts[account_index]
.try_borrow_mut()
.map_err(|_| InstructionError::AccountBorrowOutstanding)?;
pre_accounts[unique_index].verify(&program_id, &rent_collector.rent, &account)?;
pre_accounts[unique_index].verify(&program_id, rent, &account)?;
pre_sum += u128::from(pre_accounts[unique_index].lamports());
post_sum += u128::from(account.lamports);
Ok(())
@ -453,10 +425,9 @@ impl MessageProcessor {
message: &Message,
instruction: &CompiledInstruction,
pre_accounts: &mut [PreAccount],
accounts: &[Rc<RefCell<Account>>],
program_id: &Pubkey,
rent: &Rent,
signers: &[Pubkey],
accounts: &[Rc<RefCell<Account>>],
) -> Result<(), InstructionError> {
// Verify the per-account instruction results
let (mut pre_sum, mut post_sum) = (0_u128, 0_u128);
@ -471,14 +442,7 @@ impl MessageProcessor {
.try_borrow_mut()
.map_err(|_| InstructionError::AccountBorrowOutstanding)?;
pre_account.verify_cross_program(
message.is_writable(account_index),
message.is_signer(account_index),
signers,
&program_id,
&rent,
&account,
)?;
pre_account.verify(&program_id, &rent, &account)?;
pre_sum += u128::from(pre_account.lamports());
post_sum += u128::from(account.lamports);
@ -525,7 +489,7 @@ impl MessageProcessor {
&invoke_context.pre_accounts,
executable_accounts,
accounts,
rent_collector,
&rent_collector.rent,
)?;
Ok(())
}
@ -622,7 +586,6 @@ mod tests {
.verify_and_update(
&message,
&message.instructions[0],
&[],
&accounts[not_owned_index..owned_index + 1],
)
.unwrap();
@ -638,7 +601,6 @@ mod tests {
invoke_context.verify_and_update(
&message,
&message.instructions[0],
&[],
&accounts[not_owned_index..owned_index + 1],
),
Err(InstructionError::ExternalAccountDataModified)
@ -685,23 +647,16 @@ mod tests {
);
}
struct Change<'a> {
struct Change {
program_id: Pubkey,
message_is_writable: bool,
message_is_signer: bool,
signers: &'a [Pubkey],
rent: Rent,
pre: PreAccount,
post: Account,
}
impl<'a> Change<'a> {
impl Change {
pub fn new(owner: &Pubkey, program_id: &Pubkey) -> Self {
Self {
// key: Pubkey::new_rand(),
program_id: *program_id,
message_is_writable: false,
message_is_signer: false,
signers: &[],
rent: Rent::default(),
pre: PreAccount::new(
&Pubkey::new_rand(),
@ -721,26 +676,10 @@ mod tests {
},
}
}
pub fn new_cross_program(owner: &Pubkey, program_id: &Pubkey, key: &Pubkey) -> Self {
let mut change = Change::new(owner, program_id);
change.pre.key = *key;
change
}
pub fn read_only(mut self) -> Self {
self.pre.is_writable = false;
self
}
pub fn writable(mut self, pre: bool, message_is_writable: bool) -> Self {
self.pre.is_writable = pre;
self.message_is_writable = message_is_writable;
self
}
pub fn signer(mut self, pre: bool, message_is_signer: bool, signers: &'a [Pubkey]) -> Self {
self.pre.is_signer = pre;
self.message_is_signer = message_is_signer;
self.signers = signers;
self
}
pub fn executable(mut self, pre: bool, post: bool) -> Self {
self.pre.is_executable = pre;
self.post.executable = post;
@ -768,16 +707,6 @@ mod tests {
pub fn verify(&self) -> Result<(), InstructionError> {
self.pre.verify(&self.program_id, &self.rent, &self.post)
}
pub fn verify_cross_program(&self) -> Result<(), InstructionError> {
self.pre.verify_cross_program(
self.message_is_writable,
self.message_is_signer,
self.signers,
&self.program_id,
&self.rent,
&self.post,
)
}
}
#[test]
@ -940,59 +869,6 @@ mod tests {
);
}
#[test]
fn test_verify_account_changes_writable() {
let owner = Pubkey::new_rand();
let system_program_id = system_program::id();
assert_eq!(
Change::new(&owner, &system_program_id)
.writable(true, false)
.verify_cross_program(),
Ok(()),
"account can we changed to readonly"
);
assert_eq!(
Change::new(&owner, &system_program_id)
.writable(false, true)
.verify_cross_program(),
Err(InstructionError::WritableModified),
"account cannot be changed to writable"
);
}
#[test]
fn test_verify_account_changes_signer() {
let owner = Pubkey::new_rand();
let system_program_id = system_program::id();
let key = Pubkey::new_rand();
assert_eq!(
Change::new_cross_program(&owner, &system_program_id, &key)
.signer(false, true, &[key])
.verify_cross_program(),
Ok(()),
"account signed by a signer"
);
assert_eq!(
Change::new_cross_program(&owner, &system_program_id, &key)
.signer(false, true, &[])
.verify_cross_program(),
Err(InstructionError::SignerModified),
"account cannot be changed to signed if no signer"
);
assert_eq!(
Change::new_cross_program(&owner, &system_program_id, &key)
.signer(false, true, &[Pubkey::new_rand(), Pubkey::new_rand()])
.verify_cross_program(),
Err(InstructionError::SignerModified),
"account cannot be changed to signed if no signer exists"
);
}
#[test]
fn test_verify_account_changes_data_len() {
let alice_program_id = Pubkey::new_rand();
@ -1446,7 +1322,6 @@ mod tests {
&message,
&executable_accounts,
&accounts,
&[],
&mut invoke_context,
),
Err(InstructionError::ExternalAccountDataModified)
@ -1474,7 +1349,6 @@ mod tests {
&message,
&executable_accounts,
&accounts,
&[],
&mut invoke_context,
),
case.1