backport #479 to v2

This commit is contained in:
Jack May 2020-09-21 13:08:17 -07:00
parent 51dc1ae267
commit f68ff75ba0
3 changed files with 51 additions and 28 deletions

View File

@ -247,11 +247,6 @@ 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

View File

@ -56,14 +56,11 @@ 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 `DangerInitializeMultisig` instruction requires no signers and MUST be included within
/// The `InitializeMultisig` 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.
///
@ -72,7 +69,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.
DangerInitializeMultisig {
InitializeMultisig {
/// The number of signers (M) required to validate this multisignature account.
m: u8,
},
@ -353,7 +350,7 @@ impl TokenInstruction {
1 => Self::InitializeAccount,
2 => {
let &m = rest.get(0).ok_or(InvalidInstruction)?;
Self::DangerInitializeMultisig { m }
Self::InitializeMultisig { m }
}
3 | 4 | 7 | 8 => {
let amount = rest
@ -449,7 +446,7 @@ impl TokenInstruction {
Self::pack_pubkey_option(freeze_authority, &mut buf);
}
Self::InitializeAccount => buf.push(1),
&Self::DangerInitializeMultisig { m } => {
&Self::InitializeMultisig { m } => {
buf.push(2);
buf.push(m);
}
@ -624,8 +621,8 @@ pub fn initialize_account(
})
}
/// Creates a `DangerInitializeMultisig` instruction.
pub fn danger_initialize_multisig(
/// Creates a `InitializeMultisig` instruction.
pub fn initialize_multisig(
token_program_id: &Pubkey,
multisig_pubkey: &Pubkey,
signer_pubkeys: &[&Pubkey],
@ -637,7 +634,7 @@ pub fn danger_initialize_multisig(
{
return Err(ProgramError::MissingRequiredSignature);
}
let data = TokenInstruction::DangerInitializeMultisig { m }.pack();
let data = TokenInstruction::InitializeMultisig { m }.pack();
let mut accounts = Vec::with_capacity(1 + 1 + signer_pubkeys.len());
accounts.push(AccountMeta::new(*multisig_pubkey, false));
@ -1083,7 +1080,7 @@ mod test {
let unpacked = TokenInstruction::unpack(&expect).unwrap();
assert_eq!(unpacked, check);
let check = TokenInstruction::DangerInitializeMultisig { m: 1 };
let check = TokenInstruction::InitializeMultisig { m: 1 };
let packed = check.pack();
let expect = Vec::from([2u8, 1]);
assert_eq!(packed, expect);

View File

@ -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},
@ -99,8 +99,8 @@ impl Processor {
Ok(())
}
/// Processes a [DangerInitializeMultisig](enum.TokenInstruction.html) instruction.
pub fn process_danger_initialize_multisig(accounts: &[AccountInfo], m: u8) -> ProgramResult {
/// Processes a [InitializeMultisig](enum.TokenInstruction.html) instruction.
pub fn process_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();
@ -637,9 +637,9 @@ impl Processor {
info!("Instruction: InitializeAccount");
Self::process_initialize_account(accounts)
}
TokenInstruction::DangerInitializeMultisig { m } => {
info!("Instruction: DangerInitializeMultisig");
Self::process_danger_initialize_multisig(accounts, m)
TokenInstruction::InitializeMultisig { m } => {
info!("Instruction: InitializeMultisig");
Self::process_initialize_multisig(accounts, m)
}
TokenInstruction::Transfer { amount } => {
info!("Instruction: Transfer");
@ -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);
}
@ -3600,8 +3604,7 @@ mod tests {
assert_eq!(
Err(TokenError::NotRentExempt.into()),
do_process_instruction(
danger_initialize_multisig(&program_id, &multisig_key, &[&signer_keys[0]], 1)
.unwrap(),
initialize_multisig(&program_id, &multisig_key, &[&signer_keys[0]], 1).unwrap(),
vec![
&mut multisig_account,
&mut rent_sysvar,
@ -3615,7 +3618,7 @@ mod tests {
// single signer
let account_info_iter = &mut signer_accounts.iter_mut();
do_process_instruction(
danger_initialize_multisig(&program_id, &multisig_key, &[&signer_keys[0]], 1).unwrap(),
initialize_multisig(&program_id, &multisig_key, &[&signer_keys[0]], 1).unwrap(),
vec![
&mut multisig_account,
&mut rent_sysvar,
@ -3627,7 +3630,7 @@ mod tests {
// multiple signer
let account_info_iter = &mut signer_accounts.iter_mut();
do_process_instruction(
danger_initialize_multisig(
initialize_multisig(
&program_id,
&multisig_delegate_key,
&signer_key_refs,
@ -4087,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();
}
@ -4097,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]