Fix multisign check (#479)
This commit is contained in:
parent
7acae7af7d
commit
80ea6ff536
|
@ -247,6 +247,11 @@ Account's owner via the `Revoke` instruction.
|
|||
|
||||
### Multisignatures
|
||||
|
||||
Warning: SPL Token v2's multisignature validation is compromised and should not
|
||||
be used. Validation will erroneously pass a single valid signer signs m times.
|
||||
For more information:
|
||||
https://github.com/solana-labs/solana-program-library/issues/477
|
||||
|
||||
M of N multisignatures are supported and can be used in place of Mint
|
||||
authorities or Account owners or delegates. Multisignature authorities must be
|
||||
initialized with the `InitializeMultisig` instruction. Initialization specifies
|
||||
|
|
|
@ -4,7 +4,7 @@
|
|||
|
||||
use crate::{
|
||||
error::TokenError,
|
||||
instruction::{is_valid_signer_index, AuthorityType, TokenInstruction},
|
||||
instruction::{is_valid_signer_index, AuthorityType, TokenInstruction, MAX_SIGNERS},
|
||||
option::COption,
|
||||
pack::{IsInitialized, Pack},
|
||||
state::{Account, AccountState, Mint, Multisig},
|
||||
|
@ -714,14 +714,18 @@ impl Processor {
|
|||
{
|
||||
let multisig = Multisig::unpack(&owner_account_info.data.borrow())?;
|
||||
let mut num_signers = 0;
|
||||
let mut matched = [false; MAX_SIGNERS];
|
||||
for signer in signers.iter() {
|
||||
if multisig.signers[0..multisig.n as usize].contains(signer.key) {
|
||||
for (position, key) in multisig.signers[0..multisig.n as usize].iter().enumerate() {
|
||||
if key == signer.key && !matched[position] {
|
||||
if !signer.is_signer {
|
||||
return Err(ProgramError::MissingRequiredSignature);
|
||||
}
|
||||
matched[position] = true;
|
||||
num_signers += 1;
|
||||
}
|
||||
}
|
||||
}
|
||||
if num_signers < multisig.m {
|
||||
return Err(ProgramError::MissingRequiredSignature);
|
||||
}
|
||||
|
@ -4086,7 +4090,7 @@ mod tests {
|
|||
{
|
||||
let mut multisig =
|
||||
Multisig::unpack_unchecked(&owner_account_info.data.borrow()).unwrap();
|
||||
multisig.m = 2; // TODO 11?
|
||||
multisig.m = 11;
|
||||
multisig.n = 11;
|
||||
Multisig::pack(multisig, &mut owner_account_info.data.borrow_mut()).unwrap();
|
||||
}
|
||||
|
@ -4096,6 +4100,34 @@ mod tests {
|
|||
Processor::validate_owner(&program_id, &owner_key, &owner_account_info, &signers)
|
||||
);
|
||||
signers[5].is_signer = true;
|
||||
|
||||
// 11:11, single signer signs multiple times
|
||||
{
|
||||
let mut signer_lamports = 0;
|
||||
let mut signer_data = vec![];
|
||||
let signers = vec![
|
||||
AccountInfo::new(
|
||||
&signer_keys[5],
|
||||
true,
|
||||
false,
|
||||
&mut signer_lamports,
|
||||
&mut signer_data,
|
||||
&program_id,
|
||||
false,
|
||||
Epoch::default(),
|
||||
);
|
||||
MAX_SIGNERS + 1
|
||||
];
|
||||
let mut multisig =
|
||||
Multisig::unpack_unchecked(&owner_account_info.data.borrow()).unwrap();
|
||||
multisig.m = 11;
|
||||
multisig.n = 11;
|
||||
Multisig::pack(multisig, &mut owner_account_info.data.borrow_mut()).unwrap();
|
||||
assert_eq!(
|
||||
Err(ProgramError::MissingRequiredSignature),
|
||||
Processor::validate_owner(&program_id, &owner_key, &owner_account_info, &signers)
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
|
|
|
@ -56,11 +56,14 @@ pub enum TokenInstruction {
|
|||
InitializeAccount,
|
||||
/// Initializes a multisignature account with N provided signers.
|
||||
///
|
||||
/// Warning, this instruction is compromised:
|
||||
/// https://github.com/solana-labs/solana-program-library/issues/477
|
||||
///
|
||||
/// Multisignature accounts can used in place of any single owner/delegate accounts in any
|
||||
/// token instruction that require an owner/delegate to be present. The variant field represents the
|
||||
/// number of signers (M) required to validate this multisignature account.
|
||||
///
|
||||
/// The `InitializeMultisig` instruction requires no signers and MUST be included within
|
||||
/// The `DangerInitializeMultisig` instruction requires no signers and MUST be included within
|
||||
/// the same Transaction as the system program's `CreateInstruction` that creates the account
|
||||
/// being initialized. Otherwise another party can acquire ownership of the uninitialized account.
|
||||
///
|
||||
|
@ -69,7 +72,7 @@ pub enum TokenInstruction {
|
|||
/// 0. `[writable]` The multisignature account to initialize.
|
||||
/// 1. `[]` Rent sysvar
|
||||
/// 2. ..2+N. `[]` The signer accounts, must equal to N where 1 <= N <= 11.
|
||||
InitializeMultisig {
|
||||
DangerInitializeMultisig {
|
||||
/// The number of signers (M) required to validate this multisignature account.
|
||||
m: u8,
|
||||
},
|
||||
|
@ -350,7 +353,7 @@ impl TokenInstruction {
|
|||
1 => Self::InitializeAccount,
|
||||
2 => {
|
||||
let &m = rest.get(0).ok_or(InvalidInstruction)?;
|
||||
Self::InitializeMultisig { m }
|
||||
Self::DangerInitializeMultisig { m }
|
||||
}
|
||||
3 | 4 | 7 | 8 => {
|
||||
let amount = rest
|
||||
|
@ -446,7 +449,7 @@ impl TokenInstruction {
|
|||
Self::pack_pubkey_option(freeze_authority, &mut buf);
|
||||
}
|
||||
Self::InitializeAccount => buf.push(1),
|
||||
&Self::InitializeMultisig { m } => {
|
||||
&Self::DangerInitializeMultisig { m } => {
|
||||
buf.push(2);
|
||||
buf.push(m);
|
||||
}
|
||||
|
@ -621,8 +624,8 @@ pub fn initialize_account(
|
|||
})
|
||||
}
|
||||
|
||||
/// Creates a `InitializeMultisig` instruction.
|
||||
pub fn initialize_multisig(
|
||||
/// Creates a `DangerInitializeMultisig` instruction.
|
||||
pub fn danger_initialize_multisig(
|
||||
token_program_id: &Pubkey,
|
||||
multisig_pubkey: &Pubkey,
|
||||
signer_pubkeys: &[&Pubkey],
|
||||
|
@ -634,7 +637,7 @@ pub fn initialize_multisig(
|
|||
{
|
||||
return Err(ProgramError::MissingRequiredSignature);
|
||||
}
|
||||
let data = TokenInstruction::InitializeMultisig { m }.pack();
|
||||
let data = TokenInstruction::DangerInitializeMultisig { m }.pack();
|
||||
|
||||
let mut accounts = Vec::with_capacity(1 + 1 + signer_pubkeys.len());
|
||||
accounts.push(AccountMeta::new(*multisig_pubkey, false));
|
||||
|
@ -1080,7 +1083,7 @@ mod test {
|
|||
let unpacked = TokenInstruction::unpack(&expect).unwrap();
|
||||
assert_eq!(unpacked, check);
|
||||
|
||||
let check = TokenInstruction::InitializeMultisig { m: 1 };
|
||||
let check = TokenInstruction::DangerInitializeMultisig { m: 1 };
|
||||
let packed = check.pack();
|
||||
let expect = Vec::from([2u8, 1]);
|
||||
assert_eq!(packed, expect);
|
||||
|
|
|
@ -99,8 +99,8 @@ impl Processor {
|
|||
})
|
||||
}
|
||||
|
||||
/// Processes a [InitializeMultisig](enum.TokenInstruction.html) instruction.
|
||||
pub fn process_initialize_multisig(accounts: &[AccountInfo], m: u8) -> ProgramResult {
|
||||
/// Processes a [DangerInitializeMultisig](enum.TokenInstruction.html) instruction.
|
||||
pub fn process_danger_initialize_multisig(accounts: &[AccountInfo], m: u8) -> ProgramResult {
|
||||
let account_info_iter = &mut accounts.iter();
|
||||
let multisig_info = next_account_info(account_info_iter)?;
|
||||
let multisig_info_data_len = multisig_info.data_len();
|
||||
|
@ -642,9 +642,9 @@ impl Processor {
|
|||
info!("Instruction: InitializeAccount");
|
||||
Self::process_initialize_account(accounts)
|
||||
}
|
||||
TokenInstruction::InitializeMultisig { m } => {
|
||||
info!("Instruction: InitializeMultisig");
|
||||
Self::process_initialize_multisig(accounts, m)
|
||||
TokenInstruction::DangerInitializeMultisig { m } => {
|
||||
info!("Instruction: DangerInitializeMultisig");
|
||||
Self::process_danger_initialize_multisig(accounts, m)
|
||||
}
|
||||
TokenInstruction::Transfer { amount } => {
|
||||
info!("Instruction: Transfer");
|
||||
|
@ -2733,7 +2733,8 @@ mod tests {
|
|||
assert_eq!(
|
||||
Err(TokenError::NotRentExempt.into()),
|
||||
do_process_instruction(
|
||||
initialize_multisig(&program_id, &multisig_key, &[&signer_keys[0]], 1).unwrap(),
|
||||
danger_initialize_multisig(&program_id, &multisig_key, &[&signer_keys[0]], 1)
|
||||
.unwrap(),
|
||||
vec![
|
||||
&mut multisig_account,
|
||||
&mut rent_sysvar,
|
||||
|
@ -2747,7 +2748,7 @@ mod tests {
|
|||
// single signer
|
||||
let account_info_iter = &mut signer_accounts.iter_mut();
|
||||
do_process_instruction(
|
||||
initialize_multisig(&program_id, &multisig_key, &[&signer_keys[0]], 1).unwrap(),
|
||||
danger_initialize_multisig(&program_id, &multisig_key, &[&signer_keys[0]], 1).unwrap(),
|
||||
vec![
|
||||
&mut multisig_account,
|
||||
&mut rent_sysvar,
|
||||
|
@ -2759,7 +2760,7 @@ mod tests {
|
|||
// multiple signer
|
||||
let account_info_iter = &mut signer_accounts.iter_mut();
|
||||
do_process_instruction(
|
||||
initialize_multisig(
|
||||
danger_initialize_multisig(
|
||||
&program_id,
|
||||
&multisig_delegate_key,
|
||||
&signer_key_refs,
|
||||
|
|
Loading…
Reference in New Issue