More certik findings (#69)

* LIB-01 | Possible to Use Incorrect Mint When Initializing a Sale

* LIB-02 | Denial-of-Service Attack Can Prevent Legitimate Sales From Initializing

* LIB-03 | Missing Owner Validation for `sale_token_mint`

* COT-02 | Uninitialized Token Accounts
This commit is contained in:
Karl Kempe 2022-08-26 16:31:08 -05:00 committed by GitHub
parent 77e588e613
commit ce72b3a4df
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 84 additions and 55 deletions

View File

@ -154,13 +154,20 @@ pub struct Contribute<'info> {
pub buyer_token_acct: Account<'info, TokenAccount>,
#[account(
mut,
init_if_needed,
payer = owner,
associated_token::mint = accepted_mint,
associated_token::authority = custodian,
)]
/// This must be an associated token account
pub custodian_token_acct: Account<'info, TokenAccount>,
#[account(
constraint = rent.key() == rent::id() @ ContributorError::InvalidSystemProgram
)]
/// CHECK: Rent
pub rent: AccountInfo<'info>,
#[account(mut)]
pub owner: Signer<'info>,
pub system_program: Program<'info, System>,
@ -589,13 +596,25 @@ pub struct ClaimAllocation<'info> {
pub custodian_sale_token_acct: Account<'info, TokenAccount>,
#[account(
mut,
associated_token::mint = sale.sale_token_mint,
init_if_needed,
payer = owner,
associated_token::mint = sale_token_mint,
associated_token::authority = owner,
)]
/// This must be an associated token account
pub buyer_sale_token_acct: Account<'info, TokenAccount>,
#[account(
constraint = sale_token_mint.key() == sale.sale_token_mint @ ContributorError::InvalidSaleToken
)]
pub sale_token_mint: Account<'info, Mint>,
#[account(
constraint = rent.key() == rent::id() @ ContributorError::InvalidSystemProgram
)]
/// CHECK: Rent
pub rent: AccountInfo<'info>,
#[account(mut)]
pub owner: Signer<'info>,
pub system_program: Program<'info, System>,

View File

@ -124,4 +124,7 @@ pub enum ContributorError {
#[msg("InvalidSaleTokenATA")]
InvalidSaleTokenATA,
#[msg("InvalidSaleToken")]
InvalidSaleToken,
}

View File

@ -72,34 +72,45 @@ pub mod anchor_contributor {
// We assume that the conductor is sending a legitimate token, whether it is
// a Solana native token or minted by the token bridge program.
if sale.token_chain == CHAIN_ID {
// In the case that the token chain is Solana, we will attempt to deserialize the Mint
// account and be on our way. If for any reason we cannot, we will block the sale
// as a precaution.
match mint_acct_info.try_borrow_data() {
Err(_) => {
sale.block_contributions();
}
Ok(data) => {
let mut bf: &[u8] = &data;
match token::Mint::try_deserialize(&mut bf) {
Err(_) => {
sale.block_contributions();
}
Ok(mint_info) => {
// We want to save the sale token's mint information in the Sale struct. Most
// important of which is the number of decimals for this SPL token. The sale
// token that lives on the conductor chain can have a different number of decimals.
// Given how Portal works in attesting tokens, the foreign decimals will always
// be at least the amount found here.
sale.set_sale_token_mint_info(
&ctx.accounts.sale_token_mint.key(),
&mint_info,
&ctx.accounts.custodian.key(),
)?;
// Mint account passed into context needs to be correct
require!(
mint_acct_info.key().to_bytes() == sale.token_address,
ContributorError::InvalidSaleToken
);
if *mint_acct_info.owner != token::ID {
// If the Mint account is not owned by the SPL Token Program, do not trust it
sale.block_contributions();
} else {
// In the case that the token chain is Solana, we will attempt to deserialize the Mint
// account and be on our way. If for any reason we cannot, we will block the sale
// as a precaution.
match mint_acct_info.try_borrow_data() {
Err(_) => {
sale.block_contributions();
}
Ok(data) => {
let mut bf: &[u8] = &data;
match token::Mint::try_deserialize(&mut bf) {
Err(_) => {
sale.block_contributions();
}
Ok(mint_info) => {
// We want to save the sale token's mint information in the Sale struct. Most
// important of which is the number of decimals for this SPL token. The sale
// token that lives on the conductor chain can have a different number of decimals.
// Given how Portal works in attesting tokens, the foreign decimals will always
// be at least the amount found here.
sale.set_sale_token_mint_info(
&mint_acct_info.key(),
&mint_info,
&ctx.accounts.custodian.key(),
)?;
}
}
}
}
};
};
}
} else {
// In the case that the token chain isn't Solana, we will assume that the token
// has not been attestd yet if there is no account found.
@ -117,24 +128,23 @@ pub mod anchor_contributor {
&[
b"wrapped",
&sale.token_chain.to_be_bytes(),
&sale.native_sale_token_mint,
&sale.token_address,
],
&ctx.accounts.token_bridge.key(),
);
if mint != ctx.accounts.sale_token_mint.key() {
sale.block_contributions();
}
// Mint account passed into context needs to be correct
require!(
mint_acct_info.key() == mint,
ContributorError::InvalidSaleToken
);
// We want to save the sale token's mint information in the Sale struct. Most
// important of which is the number of decimals for this SPL token. The sale
// token that lives on the conductor chain can have a different number of decimals.
// Given how Portal works in attesting tokens, the foreign decimals will always
// be at least the amount found here.
sale.set_sale_token_mint_info(
&ctx.accounts.sale_token_mint.key(),
&mint_info,
&ctx.accounts.custodian.key(),
)?;
sale.set_sale_token_mint_info(&mint, &mint_info, &ctx.accounts.custodian.key())?;
}
// We need to verify that the accepted tokens are actual mints.

View File

@ -51,15 +51,15 @@ pub enum SaleStatus {
#[account]
pub struct Sale {
pub id: [u8; 32], // 32
pub native_sale_token_mint: [u8; 32], // 32 Native for sale token chain.
pub token_chain: u16, // 2
pub token_decimals: u8, // 1
pub times: SaleTimes, // SaleTimes::LEN
pub recipient: [u8; 32], // 32
pub status: SaleStatus, // 1
pub kyc_authority: [u8; 20], // 20 (this is an evm pubkey)
pub initialized: bool, // 1
pub id: [u8; 32], // 32
pub token_address: [u8; 32], // 32 Native for sale token chain.
pub token_chain: u16, // 2
pub token_decimals: u8, // 1
pub times: SaleTimes, // SaleTimes::LEN
pub recipient: [u8; 32], // 32
pub status: SaleStatus, // 1
pub kyc_authority: [u8; 20], // 20 (this is an evm pubkey)
pub initialized: bool, // 1
pub totals: Vec<AssetTotal>, // 4 + AssetTotal::LEN * ACCEPTED_TOKENS_MAX
pub native_token_decimals: u8, // 1
@ -201,7 +201,7 @@ impl Sale {
self.id = Sale::get_id(payload);
// deserialize other things
self.native_sale_token_mint.copy_from_slice(
self.token_address.copy_from_slice(
&payload[INDEX_SALE_INIT_TOKEN_ADDRESS..(INDEX_SALE_INIT_TOKEN_ADDRESS + 32)],
);
self.token_chain = to_u16_be(payload, INDEX_SALE_INIT_TOKEN_CHAIN);

View File

@ -170,6 +170,7 @@ export class IccoContributor {
systemProgram: web3.SystemProgram.programId,
buyerTokenAcct,
custodianTokenAcct,
rent: web3.SYSVAR_RENT_PUBKEY,
acceptedMint,
})
.signers([payer])
@ -430,13 +431,7 @@ export class IccoContributor {
const buyer = this.deriveBuyerAccount(saleId, payer.publicKey);
const sale = this.deriveSaleAccount(saleId);
const buyerTokenAccount = await getOrCreateAssociatedTokenAccount(
program.provider.connection,
payer,
saleTokenMint,
payer.publicKey
);
const buyerSaleTokenAcct = buyerTokenAccount.address;
const buyerSaleTokenAcct = await getAssociatedTokenAddress(saleTokenMint, payer.publicKey);
const custodianSaleTokenAcct = await getPdaAssociatedTokenAddress(saleTokenMint, custodian);
return program.methods
@ -446,6 +441,8 @@ export class IccoContributor {
sale,
buyer,
buyerSaleTokenAcct,
saleTokenMint,
rent: web3.SYSVAR_RENT_PUBKEY,
custodianSaleTokenAcct,
owner: payer.publicKey,
systemProgram: web3.SystemProgram.programId,