Fix CPI duplicate account privilege escalation (#22752)
* Adds TEST_DUPLICATE_PRIVILEGE_ESCALATION_SIGNER and TEST_DUPLICATE_PRIVILEGE_ESCALATION_WRITABLE. * Moves CPI privilege verification out of deduplication loop.
This commit is contained in:
parent
fa51e5b704
commit
a71f05f86c
|
@ -698,11 +698,23 @@ impl<'a> InvokeContext<'a> {
|
|||
);
|
||||
InstructionError::MissingAccount
|
||||
})?;
|
||||
let borrowed_account = instruction_context
|
||||
.try_borrow_account(self.transaction_context, index_in_caller)?;
|
||||
duplicate_indicies.push(deduplicated_instruction_accounts.len());
|
||||
deduplicated_instruction_accounts.push(InstructionAccount {
|
||||
index_in_transaction,
|
||||
index_in_caller,
|
||||
is_signer: account_meta.is_signer,
|
||||
is_writable: account_meta.is_writable,
|
||||
});
|
||||
}
|
||||
}
|
||||
for instruction_account in deduplicated_instruction_accounts.iter() {
|
||||
let borrowed_account = instruction_context.try_borrow_account(
|
||||
self.transaction_context,
|
||||
instruction_account.index_in_caller,
|
||||
)?;
|
||||
|
||||
// Readonly in caller cannot become writable in callee
|
||||
if account_meta.is_writable && !borrowed_account.is_writable() {
|
||||
if instruction_account.is_writable && !borrowed_account.is_writable() {
|
||||
ic_msg!(
|
||||
self,
|
||||
"{}'s writable privilege escalated",
|
||||
|
@ -713,9 +725,8 @@ impl<'a> InvokeContext<'a> {
|
|||
|
||||
// To be signed in the callee,
|
||||
// it must be either signed in the caller or by the program
|
||||
if account_meta.is_signer
|
||||
&& !(borrowed_account.is_signer()
|
||||
|| signers.contains(borrowed_account.get_key()))
|
||||
if instruction_account.is_signer
|
||||
&& !(borrowed_account.is_signer() || signers.contains(borrowed_account.get_key()))
|
||||
{
|
||||
ic_msg!(
|
||||
self,
|
||||
|
@ -724,15 +735,6 @@ impl<'a> InvokeContext<'a> {
|
|||
);
|
||||
return Err(InstructionError::PrivilegeEscalation);
|
||||
}
|
||||
|
||||
duplicate_indicies.push(deduplicated_instruction_accounts.len());
|
||||
deduplicated_instruction_accounts.push(InstructionAccount {
|
||||
index_in_transaction,
|
||||
index_in_caller,
|
||||
is_signer: account_meta.is_signer,
|
||||
is_writable: account_meta.is_writable,
|
||||
});
|
||||
}
|
||||
}
|
||||
let instruction_accounts: Vec<InstructionAccount> = duplicate_indicies
|
||||
.into_iter()
|
||||
|
|
|
@ -29,6 +29,8 @@ static const uint8_t TEST_EXECUTABLE_LAMPORTS = 16;
|
|||
static const uint8_t TEST_CALL_PRECOMPILE = 17;
|
||||
static const uint8_t ADD_LAMPORTS = 18;
|
||||
static const uint8_t TEST_RETURN_DATA_TOO_LARGE = 19;
|
||||
static const uint8_t TEST_DUPLICATE_PRIVILEGE_ESCALATION_SIGNER = 20;
|
||||
static const uint8_t TEST_DUPLICATE_PRIVILEGE_ESCALATION_WRITABLE = 21;
|
||||
|
||||
static const int MINT_INDEX = 0;
|
||||
static const int ARGUMENT_INDEX = 1;
|
||||
|
@ -611,6 +613,42 @@ extern uint64_t entrypoint(const uint8_t *input) {
|
|||
sol_set_return_data(NULL, 1027);
|
||||
break;
|
||||
}
|
||||
case TEST_DUPLICATE_PRIVILEGE_ESCALATION_SIGNER: {
|
||||
sol_log("Test duplicate privilege escalation signer");
|
||||
SolAccountMeta arguments[] = {
|
||||
{accounts[DERIVED_KEY3_INDEX].key, false, false},
|
||||
{accounts[DERIVED_KEY3_INDEX].key, false, false},
|
||||
{accounts[DERIVED_KEY3_INDEX].key, false, false}};
|
||||
uint8_t data[] = {VERIFY_PRIVILEGE_ESCALATION};
|
||||
const SolInstruction instruction = {accounts[INVOKED_PROGRAM_INDEX].key,
|
||||
arguments, SOL_ARRAY_SIZE(arguments),
|
||||
data, SOL_ARRAY_SIZE(data)};
|
||||
sol_assert(SUCCESS ==
|
||||
sol_invoke(&instruction, accounts, SOL_ARRAY_SIZE(accounts)));
|
||||
|
||||
// Signer privilege escalation will always fail the whole transaction
|
||||
instruction.accounts[1].is_signer = true;
|
||||
sol_invoke(&instruction, accounts, SOL_ARRAY_SIZE(accounts));
|
||||
break;
|
||||
}
|
||||
case TEST_DUPLICATE_PRIVILEGE_ESCALATION_WRITABLE: {
|
||||
sol_log("Test duplicate privilege escalation writable");
|
||||
SolAccountMeta arguments[] = {
|
||||
{accounts[DERIVED_KEY3_INDEX].key, false, false},
|
||||
{accounts[DERIVED_KEY3_INDEX].key, false, false},
|
||||
{accounts[DERIVED_KEY3_INDEX].key, false, false}};
|
||||
uint8_t data[] = {VERIFY_PRIVILEGE_ESCALATION};
|
||||
const SolInstruction instruction = {accounts[INVOKED_PROGRAM_INDEX].key,
|
||||
arguments, SOL_ARRAY_SIZE(arguments),
|
||||
data, SOL_ARRAY_SIZE(data)};
|
||||
sol_assert(SUCCESS ==
|
||||
sol_invoke(&instruction, accounts, SOL_ARRAY_SIZE(accounts)));
|
||||
|
||||
// Writable privilege escalation will always fail the whole transaction
|
||||
instruction.accounts[1].is_writable = true;
|
||||
sol_invoke(&instruction, accounts, SOL_ARRAY_SIZE(accounts));
|
||||
break;
|
||||
}
|
||||
|
||||
default:
|
||||
sol_panic();
|
||||
|
|
|
@ -19,6 +19,8 @@ pub const TEST_EXECUTABLE_LAMPORTS: u8 = 16;
|
|||
pub const TEST_CALL_PRECOMPILE: u8 = 17;
|
||||
pub const ADD_LAMPORTS: u8 = 18;
|
||||
pub const TEST_RETURN_DATA_TOO_LARGE: u8 = 19;
|
||||
pub const TEST_DUPLICATE_PRIVILEGE_ESCALATION_SIGNER: u8 = 20;
|
||||
pub const TEST_DUPLICATE_PRIVILEGE_ESCALATION_WRITABLE: u8 = 21;
|
||||
|
||||
pub const MINT_INDEX: usize = 0;
|
||||
pub const ARGUMENT_INDEX: usize = 1;
|
||||
|
|
|
@ -426,7 +426,6 @@ fn process_instruction(
|
|||
|
||||
// Writable privilege escalation will always fail the whole transaction
|
||||
invoked_instruction.accounts[0].is_writable = true;
|
||||
|
||||
invoke(&invoked_instruction, accounts)?;
|
||||
}
|
||||
TEST_PPROGRAM_NOT_EXECUTABLE => {
|
||||
|
@ -638,6 +637,40 @@ fn process_instruction(
|
|||
TEST_RETURN_DATA_TOO_LARGE => {
|
||||
set_return_data(&[1u8; 1028]);
|
||||
}
|
||||
TEST_DUPLICATE_PRIVILEGE_ESCALATION_SIGNER => {
|
||||
msg!("Test duplicate privilege escalation signer");
|
||||
let mut invoked_instruction = create_instruction(
|
||||
*accounts[INVOKED_PROGRAM_INDEX].key,
|
||||
&[
|
||||
(accounts[DERIVED_KEY3_INDEX].key, false, false),
|
||||
(accounts[DERIVED_KEY3_INDEX].key, false, false),
|
||||
(accounts[DERIVED_KEY3_INDEX].key, false, false),
|
||||
],
|
||||
vec![VERIFY_PRIVILEGE_ESCALATION],
|
||||
);
|
||||
invoke(&invoked_instruction, accounts)?;
|
||||
|
||||
// Signer privilege escalation will always fail the whole transaction
|
||||
invoked_instruction.accounts[1].is_signer = true;
|
||||
invoke(&invoked_instruction, accounts)?;
|
||||
}
|
||||
TEST_DUPLICATE_PRIVILEGE_ESCALATION_WRITABLE => {
|
||||
msg!("Test duplicate privilege escalation writable");
|
||||
let mut invoked_instruction = create_instruction(
|
||||
*accounts[INVOKED_PROGRAM_INDEX].key,
|
||||
&[
|
||||
(accounts[DERIVED_KEY3_INDEX].key, false, false),
|
||||
(accounts[DERIVED_KEY3_INDEX].key, false, false),
|
||||
(accounts[DERIVED_KEY3_INDEX].key, false, false),
|
||||
],
|
||||
vec![VERIFY_PRIVILEGE_ESCALATION],
|
||||
);
|
||||
invoke(&invoked_instruction, accounts)?;
|
||||
|
||||
// Writable privilege escalation will always fail the whole transaction
|
||||
invoked_instruction.accounts[1].is_writable = true;
|
||||
invoke(&invoked_instruction, accounts)?;
|
||||
}
|
||||
_ => panic!(),
|
||||
}
|
||||
|
||||
|
|
|
@ -1116,6 +1116,18 @@ fn test_program_bpf_invoke_sanity() {
|
|||
&[],
|
||||
);
|
||||
|
||||
do_invoke_failure_test_local(
|
||||
TEST_DUPLICATE_PRIVILEGE_ESCALATION_SIGNER,
|
||||
TransactionError::InstructionError(0, InstructionError::PrivilegeEscalation),
|
||||
&[invoked_program_id.clone()],
|
||||
);
|
||||
|
||||
do_invoke_failure_test_local(
|
||||
TEST_DUPLICATE_PRIVILEGE_ESCALATION_WRITABLE,
|
||||
TransactionError::InstructionError(0, InstructionError::PrivilegeEscalation),
|
||||
&[invoked_program_id.clone()],
|
||||
);
|
||||
|
||||
// Check resulting state
|
||||
|
||||
assert_eq!(43, bank.get_balance(&derived_key1));
|
||||
|
|
Loading…
Reference in New Issue