Skip to content
This repository was archived by the owner on Feb 13, 2025. It is now read-only.

Commit fa8f8a1

Browse files
febofebo
committed
Update safety comments
1 parent bf5c68b commit fa8f8a1

18 files changed

+140
-73
lines changed

‎program/src/processor/amount_to_ui_amount.rs‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,16 +23,18 @@ pub fn process_amount_to_ui_amount(
2323

2424
let mint_info = accounts.first().ok_or(ProgramError::NotEnoughAccountKeys)?;
2525
check_account_owner(mint_info)?;
26-
// SAFETY: there is a single immutable borrow of the `Mint` account data.
26+
// SAFETY: there is a single immutable borrow of the `Mint` account data. The `load`
27+
// validates that the mint is initialized.
2728
let mint = unsafe {
2829
load::<Mint>(mint_info.borrow_data_unchecked()).map_err(|_| TokenError::InvalidMint)?
2930
};
3031

3132
let mut logger = Logger::<MAX_FORMATTED_DIGITS>::default();
3233
logger.append_with_args(amount, &[Argument::Precision(mint.decimals)]);
34+
3335
// "Extract" the formatted string from the logger.
3436
//
35-
// SAFETY: the logger is guaranteed to be a valid UTF-8 string.
37+
// SAFETY: the logger is guaranteed to have a valid UTF-8 string.
3638
let mut s = unsafe { from_utf8_unchecked(&logger) };
3739

3840
if mint.decimals > 0 && s.contains('.') {

‎program/src/processor/close_account.rs‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -26,9 +26,9 @@ pub fn process_close_account(accounts: &[AccountInfo]) -> ProgramResult {
2626
return Err(ProgramError::InvalidAccountData);
2727
}
2828

29-
// SAFETY: scoped immutable borrow of the source account data. Changes to the
30-
// account info data happens after the borrow ends.
3129
{
30+
// SAFETY: scoped immutable borrow of the `source_account_info` account data. The `load`
31+
// validates that the mint is initialized.
3232
let source_account =
3333
unsafe { load::<Account>(source_account_info.borrow_data_unchecked())? };
3434

@@ -48,10 +48,12 @@ pub fn process_close_account(accounts: &[AccountInfo]) -> ProgramResult {
4848
}
4949

5050
let destination_starting_lamports = destination_account_info.lamports();
51-
// SAFETY: there are no other borrows of the source and destination accounts
52-
// at this point.
51+
52+
// Moves the lamports to the destination account.
53+
54+
// SAFETY: single mutable borrow of `destination_account_info` lamports and
55+
// there are no "active" borrows of `source_account_info` account data.
5356
unsafe {
54-
// Moves the lamports to the destination account.
5557
*destination_account_info.borrow_mut_lamports_unchecked() = destination_starting_lamports
5658
.checked_add(source_account_info.lamports())
5759
.ok_or(TokenError::Overflow)?;

‎program/src/processor/get_account_data_size.rs‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,9 +16,8 @@ pub fn process_get_account_data_size(accounts: &[AccountInfo]) -> ProgramResult
1616

1717
// Make sure the mint is valid.
1818
check_account_owner(mint_info)?;
19-
// SAFETY: there is a single immutable borrow of the `Mint` account data.
20-
//
21-
// The `Mint` account is loaded to check if it is initialized.
19+
// SAFETY: single immutable borrow of the `Mint` account data. The `load`
20+
// validates that the mint is initialized.
2221
let _ = unsafe {
2322
load::<Mint>(mint_info.borrow_data_unchecked()).map_err(|_| TokenError::InvalidMint)?
2423
};

‎program/src/processor/initialize_account2.rs‎

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,9 @@
1-
use pinocchio::{account_info::AccountInfo, pubkey::Pubkey, ProgramResult};
1+
use pinocchio::{
2+
account_info::AccountInfo,
3+
program_error::ProgramError,
4+
pubkey::{Pubkey, PUBKEY_BYTES},
5+
ProgramResult,
6+
};
27

38
use super::shared;
49

@@ -7,6 +12,13 @@ pub fn process_initialize_account2(
712
accounts: &[AccountInfo],
813
instruction_data: &[u8],
914
) -> ProgramResult {
10-
let owner = unsafe { &*(instruction_data.as_ptr() as *const Pubkey) };
15+
// SAFETY: validate `instruction_data` length.
16+
let owner = unsafe {
17+
if instruction_data.len() != PUBKEY_BYTES {
18+
return Err(ProgramError::InvalidInstructionData);
19+
} else {
20+
&*(instruction_data.as_ptr() as *const Pubkey)
21+
}
22+
};
1123
shared::initialize_account::process_initialize_account(accounts, Some(owner), true)
1224
}

‎program/src/processor/initialize_account3.rs‎

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,9 @@
1-
use pinocchio::{account_info::AccountInfo, pubkey::Pubkey, ProgramResult};
1+
use pinocchio::{
2+
account_info::AccountInfo,
3+
program_error::ProgramError,
4+
pubkey::{Pubkey, PUBKEY_BYTES},
5+
ProgramResult,
6+
};
27

38
use super::shared;
49

@@ -7,6 +12,13 @@ pub fn process_initialize_account3(
712
accounts: &[AccountInfo],
813
instruction_data: &[u8],
914
) -> ProgramResult {
10-
let owner = unsafe { &*(instruction_data.as_ptr() as *const Pubkey) };
15+
// SAFETY: validate `instruction_data` length.
16+
let owner = unsafe {
17+
if instruction_data.len() != PUBKEY_BYTES {
18+
return Err(ProgramError::InvalidInstructionData);
19+
} else {
20+
&*(instruction_data.as_ptr() as *const Pubkey)
21+
}
22+
};
1123
shared::initialize_account::process_initialize_account(accounts, Some(owner), false)
1224
}

‎program/src/processor/initialize_immutable_owner.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ use token_interface::{
77
#[inline(always)]
88
pub fn process_initialize_immutable_owner(accounts: &[AccountInfo]) -> ProgramResult {
99
let token_account_info = accounts.first().ok_or(ProgramError::NotEnoughAccountKeys)?;
10-
// SAFETY: there is a single immutable borrow of the `Account` account data.
10+
// SAFETY: single immutable borrow of the `Account` account data.
1111
let account = unsafe { load_unchecked::<Account>(token_account_info.borrow_data_unchecked())? };
1212

1313
if account.is_initialized() {

‎program/src/processor/mod.rs‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,9 @@ fn check_account_owner(account_info: &AccountInfo) -> ProgramResult {
9393
}
9494

9595
/// Validates owner(s) are present
96+
///
97+
/// Note that `owner_account_info` will be immutable borrowed when it represents
98+
/// a multisig account.
9699
#[inline(always)]
97100
fn validate_owner(
98101
expected_owner: &Pubkey,
@@ -107,8 +110,7 @@ fn validate_owner(
107110
&& owner_account_info.owner() == &TOKEN_PROGRAM_ID
108111
{
109112
// SAFETY: the caller guarantees that there are no mutable borrows of `owner_account_info`
110-
// account data, so it is ok to have multiple immutable borrows (this would normally only
111-
// happen if the account/mint is the same as the owner).
113+
// account data. The `load` validates that the account is initialized and data length.
112114
let multisig = unsafe { load::<Multisig>(owner_account_info.borrow_data_unchecked())? };
113115

114116
let mut num_signers = 0;

‎program/src/processor/revoke.rs‎

Lines changed: 16 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
use pinocchio::{account_info::AccountInfo, program_error::ProgramError, ProgramResult};
22
use token_interface::{
33
error::TokenError,
4-
state::{account::Account, load_mut},
4+
state::{account::Account, load, load_mut_unchecked},
55
};
66

77
use super::validate_owner;
@@ -11,17 +11,24 @@ pub fn process_revoke(accounts: &[AccountInfo], _instruction_data: &[u8]) -> Pro
1111
let [source_account_info, owner_info, remaning @ ..] = accounts else {
1212
return Err(ProgramError::NotEnoughAccountKeys);
1313
};
14-
// SAFETY: there are no other borrows of the `source_account` data.
15-
let source_account =
16-
unsafe { load_mut::<Account>(source_account_info.borrow_mut_data_unchecked())? };
1714

18-
if source_account.is_frozen() {
19-
return Err(TokenError::AccountFrozen.into());
20-
}
15+
{
16+
// SAFETY: scoped immutable borrow of the `source_account` account data. The `load`
17+
// validates that the source account is initialized.
18+
let source_account =
19+
unsafe { load::<Account>(source_account_info.borrow_data_unchecked())? };
2120

22-
// TODO: can source == owner?
23-
validate_owner(&source_account.owner, owner_info, remaning)?;
21+
if source_account.is_frozen() {
22+
return Err(TokenError::AccountFrozen.into());
23+
}
2424

25+
validate_owner(&source_account.owner, owner_info, remaning)?;
26+
}
27+
28+
// SAFETY: single mutable borrow of the `source_account_info` account data. The
29+
// `source_account_info` is guaranteed to be initialized.
30+
let source_account =
31+
unsafe { load_mut_unchecked::<Account>(source_account_info.borrow_mut_data_unchecked())? };
2532
source_account.clear_delegate();
2633
source_account.set_delegated_amount(0);
2734

‎program/src/processor/set_authority.rs‎

Lines changed: 37 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ use pinocchio::{
66
use token_interface::{
77
error::TokenError,
88
instruction::AuthorityType,
9-
state::{account::Account, load_mut, mint::Mint, RawType},
9+
state::{account::Account, load, load_mut_unchecked, mint::Mint, RawType},
1010
};
1111

1212
use super::validate_owner;
@@ -27,18 +27,24 @@ pub fn process_set_authority(accounts: &[AccountInfo], instruction_data: &[u8])
2727
};
2828

2929
if account_info.data_len() == Account::LEN {
30-
// SAFETY: there are no other active borrows of the `account` data.
31-
let account = unsafe { load_mut::<Account>(account_info.borrow_mut_data_unchecked())? };
30+
// SAFETY: immutable borrow of `account_info` account data. The `load`
31+
// validates that the account is initialized.
32+
let account = unsafe { load::<Account>(account_info.borrow_data_unchecked())? };
3233

3334
if account.is_frozen() {
3435
return Err(TokenError::AccountFrozen.into());
3536
}
3637

3738
match authority_type {
3839
AuthorityType::AccountOwner => {
39-
// TODO: Can account and authority be the same?
4040
validate_owner(&account.owner, authority_info, remaning)?;
4141

42+
// SAFETY: single mutable borrow of `account_info` account data. The
43+
// `account_info` is guaranteed to the initialized.
44+
let account = unsafe {
45+
load_mut_unchecked::<Account>(account_info.borrow_mut_data_unchecked())?
46+
};
47+
4248
if let Some(authority) = new_authority {
4349
account.owner = *authority;
4450
} else {
@@ -54,9 +60,15 @@ pub fn process_set_authority(accounts: &[AccountInfo], instruction_data: &[u8])
5460
}
5561
AuthorityType::CloseAccount => {
5662
let authority = account.close_authority().unwrap_or(&account.owner);
57-
// TODO: Can account and authority be the same?
63+
5864
validate_owner(authority, authority_info, remaning)?;
5965

66+
// SAFETY: single mutable borrow of `account_info` account data. The
67+
// `account_info` is guaranteed to the initialized.
68+
let account = unsafe {
69+
load_mut_unchecked::<Account>(account_info.borrow_mut_data_unchecked())?
70+
};
71+
6072
if let Some(authority) = new_authority {
6173
account.set_close_authority(authority);
6274
} else {
@@ -68,17 +80,24 @@ pub fn process_set_authority(accounts: &[AccountInfo], instruction_data: &[u8])
6880
}
6981
}
7082
} else if account_info.data_len() == Mint::LEN {
71-
// SAFETY: there are no other active borrows of the `account` data.
72-
let mint = unsafe { load_mut::<Mint>(account_info.borrow_mut_data_unchecked())? };
83+
// SAFETY: immutable borrow of `account_info` account data. The `load`
84+
// validates that the mint is initialized.
85+
let mint = unsafe { load::<Mint>(account_info.borrow_mut_data_unchecked())? };
7386

7487
match authority_type {
7588
AuthorityType::MintTokens => {
7689
// Once a mint's supply is fixed, it cannot be undone by setting a new
7790
// mint_authority.
7891
let mint_authority = mint.mint_authority().ok_or(TokenError::FixedSupply)?;
79-
// TODO: Can account and authority be the same?
92+
8093
validate_owner(mint_authority, authority_info, remaning)?;
8194

95+
// SAFETY: single mutable borrow of `account_info` account data. The
96+
// `account_info` is guaranteed to the initialized.
97+
let mint = unsafe {
98+
load_mut_unchecked::<Mint>(account_info.borrow_mut_data_unchecked())?
99+
};
100+
82101
if let Some(authority) = new_authority {
83102
mint.set_mint_authority(authority);
84103
} else {
@@ -91,9 +110,15 @@ pub fn process_set_authority(accounts: &[AccountInfo], instruction_data: &[u8])
91110
let freeze_authority = mint
92111
.freeze_authority()
93112
.ok_or(TokenError::MintCannotFreeze)?;
94-
// TODO: Can account and authority be the same?
113+
95114
validate_owner(freeze_authority, authority_info, remaning)?;
96115

116+
// SAFETY: single mutable borrow of `account_info` account data. The
117+
// `account_info` is guaranteed to the initialized.
118+
let mint = unsafe {
119+
load_mut_unchecked::<Mint>(account_info.borrow_mut_data_unchecked())?
120+
};
121+
97122
if let Some(authority) = new_authority {
98123
mint.set_freeze_authority(authority);
99124
} else {
@@ -123,7 +148,7 @@ impl SetAuthority<'_> {
123148
// The minimum expected size of the instruction data.
124149
// - authority_type (1 byte)
125150
// - option + new_authority (1 byte + 32 bytes)
126-
if bytes.len() < 2 {
151+
if bytes.len() < 2 || (bytes[1] == 1 && bytes.len() < 34) {
127152
return Err(ProgramError::InvalidInstructionData);
128153
}
129154

@@ -135,11 +160,13 @@ impl SetAuthority<'_> {
135160

136161
#[inline(always)]
137162
pub fn authority_type(&self) -> Result<AuthorityType, ProgramError> {
163+
// SAFETY: `bytes` length is validated in `try_from_bytes`.
138164
unsafe { AuthorityType::from(*self.raw) }
139165
}
140166

141167
#[inline(always)]
142168
pub fn new_authority(&self) -> Option<&Pubkey> {
169+
// SAFETY: `bytes` length is validated in `try_from_bytes`.
143170
unsafe {
144171
if *self.raw.add(1) == 0 {
145172
Option::None

‎program/src/processor/shared/approve.rs‎

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
use pinocchio::{account_info::AccountInfo, program_error::ProgramError, ProgramResult};
22
use token_interface::{
33
error::TokenError,
4-
state::{account::Account, load, load_mut, mint::Mint},
4+
state::{account::Account, load, load_mut_unchecked, mint::Mint},
55
};
66

77
use crate::processor::validate_owner;
@@ -45,9 +45,8 @@ pub fn process_approve(
4545

4646
// Validates source account.
4747
{
48-
// SAFETY: scoped immutable borrow of `source_account_info` account data. When
49-
// `owner_info` is the same as `source_account_info`, there will be another immutable
50-
// borrow in `validate_owner` – this is safe because both borrows are immutable.
48+
// SAFETY: scoped immutable borrow of `source_account_info` account data. The `load`
49+
// validates that the account is initialized.
5150
let source_account =
5251
unsafe { load::<Account>(source_account_info.borrow_data_unchecked())? };
5352

@@ -60,6 +59,8 @@ pub fn process_approve(
6059
return Err(TokenError::MintMismatch.into());
6160
}
6261

62+
// SAFETY: scoped immutable borrow of `mint_info` account data. The `load`
63+
// validates that the account is initialized.
6364
let mint = unsafe { load::<Mint>(mint_info.borrow_data_unchecked())? };
6465

6566
if expected_decimals != mint.decimals {
@@ -72,10 +73,10 @@ pub fn process_approve(
7273

7374
// Sets the delegate and delegated amount.
7475

75-
// SAFETY: any immutable borrow of `source_account_info` account data is dropped at
76-
// this point, so it is safe to borrow mutably.
76+
// SAFETY: there is a single mutable borrow to `source_account_info` account data.
77+
// The account is also guaranteed to be initialized.
7778
let source_account =
78-
unsafe { load_mut::<Account>(source_account_info.borrow_mut_data_unchecked())? };
79+
unsafe { load_mut_unchecked::<Account>(source_account_info.borrow_mut_data_unchecked())? };
7980
source_account.set_delegate(delegate_info.key());
8081
source_account.set_delegated_amount(amount);
8182

0 commit comments

Comments
 (0)