Accounting error in batched instructions
During the audit the client surfaced an issue in the batch processing of instructions.
The new implementation of the token program, p-token, added the ability to process instructions in a batch. The Solana runtime provides some safety guarantees involving state changes to accounts, that are only checked after a transaction has been executed but not finalized. If the state of an account differs from before the transaction, to after the transaction, ownership checks are performed that ensure that account data has not been modified by programs that should not have the ability to. During batch processing, this check can be bypassed for certain instructions like transfer which lack explicit ownership checks.
if self_transfer || amount == 0 {
// Validates the token accounts owner since we are not writing
// to these account.
check_account_owner(source_account_info)?;
check_account_owner(destination_account_info)?;
} else {
// Moves the tokens.
source_account.set_amount(remaining_amount);
// SAFETY: single mutable borrow to `destination_account_info` account data; the
// account is guaranteed to be initialized and different than
// `source_account_info`; it was also already validated to be a token
// account.
let destination_account = unsafe {
load_mut_unchecked::<Account>(destination_account_info.borrow_mut_data_unchecked())?
};
// Note: The amount of a token account is always within the range of the
// mint supply (`u64`).
destination_account.set_amount(destination_account.amount() + amount);
if source_account.is_native() {
// SAFETY: single mutable borrow to `source_account_info` lamports.
let source_lamports = unsafe { source_account_info.borrow_mut_lamports_unchecked() };
*source_lamports = source_lamports
.checked_sub(amount)
.ok_or(TokenError::Overflow)?;
// SAFETY: single mutable borrow to `destination_account_info` lamports; the
// account is already validated to be different from
// `source_account_info`.
let destination_lamports =
unsafe { destination_account_info.borrow_mut_lamports_unchecked() };
*destination_lamports = destination_lamports
.checked_add(amount)
.ok_or(TokenError::Overflow)?;
}
}Here, explicit ownership checks are only performed for self transfers or if the transfered amount equals 0. The special handling of "native" accounts could be abused by batch processing transfers involving an account that is attacker controlled and not owned by the token program. As long as the final state of the attacker controlled account exactly matches the state before the transaction, the runtime will not verify ownership and lead to accounting errors.