NfNode Rewards Program - Wayru

A user holding an NFT that was previously minted can initialize a new NfNode entry using the init_nfnode instruction. To do so, the user must provide several accounts, including the nft_mint_address, user_nft_token_account, host, manufacturer, among others. This instruction initializes an nfnode_entry, which is a Program Derived Address (PDA) composed of a fixed seed and a variable component, specifically the nft_mint_address.

The owner of this nfnode_entry, who created it, will be able to claim their corresponding rewards daily using the owner_claim_rewards instruction. Similarly, the host or manufacturer assigned by the owner during the creation of the nfnode_entry can claim their corresponding rewards daily using the other_claim_rewards instruction. Furthermore, the owner of the nfnode_entry can change the address of the host at any time.

However, the init_nfnode, update_nfnode, and owner_claim_rewards instructions do not verify that the signing user is the authority over the user_nft_token_account, nor do the latter two instructions verify that the signer is the actual owner who created the nfnode_entry.

This lack of verification results in the following vulnerabilities:

1. Unauthorized Initialization:
   Any user who knows the user_nft_token_account address of the legitimate NFT owner can initialize an nfnode_entry by providing that user_nft_token_account and assigning its own address as the host. This would allow the attacker to subsequently claim rewards both as the host and as the owner, while the legitimate NFT owner would no longer be able to create an nfnode_entry.

   
   

2. Unauthorized Updates:
   Any user can update the nfnode_entry of a legitimate user using the update_nfnode instruction by providing the user_nft_token_account address of the legitimate user who created the nfnode_entry and assigning its own address as the new host. This would allow the attacker to subsequently claim rewards legitimately using the other_claim_rewards instruction.

   
   

3. Unauthorized Reward Claims:
   Any user can claim daily rewards before the legitimate owner of the nfnode_entry by providing the user_nft_token_account address of the legitimate user who created the nfnode_entry. This would prevent the rightful owner from claiming their rewards for that day.

The mentioned operations require partial approval from the admin, as their signature is necessary. However, if the admin does not have an external system outside the program to verify:

• That the signer is legitimately the owner of the NFT in the provided user_nft_token_account.

• That the signer is the original creator of the nfnode_entry.

The admin will lack the ability to verify which signers are legitimate, potentially resulting in the approval of transactions that introduce critical vulnerabilities. These include:

• Reward theft from a legitimate nfnode_entry

• Unauthorized updates to an existing nfnode_entry

• Creation of fraudulent nfnode_entry entries using other user's nft token account. Consequently, rightful NFT owners may be prevented from initializing their entries, compromising the integrity and security of the system.

init_nfnode.rs:

pub struct InitializeNfNode<'info> {
    #[account(mut)]
    pub user_admin: Signer<'info>,
    #[account(mut)]
    pub user: Signer<'info>,
    ///CHECK: only read account
    pub host: AccountInfo<'info>,
    ///CHECK: only read account
    pub manufacturer: AccountInfo<'info>,
    ///CHECK: only read account
    pub nft_mint_address: InterfaceAccount<'info, Mint2022>,
    /// CHECK: used to check nft ownership
    pub user_nft_token_account: AccountInfo<'info>,
    #[account(
        init,
        payer = user,
        space = 8 + std::mem::size_of::<NfNodeEntry>(),
        seeds = [b"nfnode_entry", nft_mint_address.key().as_ref()],
        bump
    )]
    pub nfnode_entry: Account<'info, NfNodeEntry>,

update_nfnode.rs:

pub struct UpdateNfNode<'info> {
    #[account(mut)]
    pub user_admin: Signer<'info>,
    #[account(mut)]
    pub user: Signer<'info>,    //TODO: no parece necesario
    ///CHECK: only read account
    pub host: AccountInfo<'info>,
    ///CHECK: only read account
    pub nft_mint_address: InterfaceAccount<'info, Mint2022>,
    /// CHECK: used to check nft ownership
    pub user_nft_token_account: AccountInfo<'info>,

owner_claim_rewards.rs:

pub struct OwnerClaimRewards<'info> {
    /// CHECK:
    #[account(mut)]
    pub user_admin: Signer<'info>,
    #[account(mut)]
    pub user: Signer<'info>,
    /// CHECK:
    pub nft_mint_address: InterfaceAccount<'info, Mint2022>,
    #[account(
        init_if_needed,
        payer = user,
        space = 8 + std::mem::size_of::<RewardEntry>(),
        seeds = [b"reward_entry", user.key().as_ref(), nft_mint_address.key().as_ref()],
        bump
    )]
    pub reward_entry: Account<'info, RewardEntry>,
    #[account(
        mut,
        seeds = [b"nfnode_entry", nft_mint_address.key().as_ref()],
        bump
    )]
    pub nfnode_entry: Box<Account<'info, NfNodeEntry>>,
    pub token_mint: Account<'info, Mint>,
    /// CHECK:
    #[account(mut, seeds = [b"token_storage"], bump)]
    pub token_storage_authority: AccountInfo<'info>,
    #[account(
        mut,
        associated_token::mint = token_mint,
        associated_token::authority = token_storage_authority,
    )]
    pub token_storage_account: Box<Account<'info, TokenAccount>>,
    #[account(
        init_if_needed,
        payer = user,
        associated_token::mint = token_mint,
        associated_token::authority = user
    )]
    pub user_token_account: Box<Account<'info, TokenAccount>>,
    /// CHECK: used to check nft ownership
    pub user_nft_token_account: AccountInfo<'info>,

Steps to reproduce the attacks:

Unauthorized Initialization:

1. Tom has a nft

2. Bob, knowing Tom's nft token account, init a nfNode entry providing Tom's token account with the nft

3. The Token Storage is funded

4. Bob claims rewards since is the owner of the nfNode entry that initialized previously

Unauthorized Updates:

1. A legitimate user who has a nft initialize the nfNode entry setting the corresponding host and manufacturer

2. The Token Storage is funded

3. A malicious user (Alice) updated the nfNode entry providing the user's nft token account and setting its own address as host

4. Alice claims rewards as legitimate host ( other_claim_rewards)
   

Unauthorized Reward Claims:

1. A legitimate user who has a nft initialize the nfNode entry setting the corresponding host and manufacturer

2. The Token Storage is funded

3. A malicious user (Alice) claim rewards providing the user's nft token account ( owner_claim_rewards).

To address this issue, it is recommended to implement the following measures:

• Add an "owner" field in nfnode_entry to hold the user's address who initializes.

• Add a validation in update_nfnode and owner_claim_rewards to ensure the user who is signing is the owner of the provided nfnode_entry.

• Add a validation in init_nfnode, update_nfnode and owner_claim_rewardsto ensure the user who is signing is the authority of the user_nft_token_account provided.

SOLVED: The Wayru team solved this issue by implementing validation mechanisms in the init_nfnode, update_nfnode, and owner_claim_rewards instructions. These validations ensure that the user_nft_token_account provided corresponds to the signer's Associated Token Account (ATA) for the specified NFT mint of the nfnode. Combined with the existing validation that checks the ATA's token balance is greater than zero, these measures ensure that only the current holder of the NFT can execute these instructions, alongside the administrator's partial signature.

The update_admin instruction allows the current system administrator to transfer ownership and designate a new admin. This admin role is critical within this Solana program, as its partial signature is required for operations such as initializing and updating the NF node and claiming rewards.

However, the instruction does not validate the public key provided as the new admin parameter. As a result, the following scenarios could occur:

• Reassigning the Admin to the Current Admin
  The current admin could mistakenly assign the role to itself. While this does not disrupt the program’s functionality, it incurs an unnecessary transaction cost without achieving any effect.

• Assigning an Invalid Address
  If an invalid or non-existent address is assigned as the new admin, it would render most subsequent instructions inoperable due to the inability to obtain the required admin signature. This effectively creates a Denial-of-Service (DoS) scenario for critical operations dependent on the admin’s authorization.

• Assigning an Incorrect or Malicious Address
  If the admin role is mistakenly or maliciously transferred to an unintended address, such as a malicious user’s account, this entity could exploit their new administrative privileges to compromise the program’s integrity.

update_admin.rs:

pub fn update_admin(ctx: Context<UpdateAdmin>, new_admin_pubkey: Pubkey) -> Result<()> {
   
    let admin_account = &mut ctx.accounts.admin_account;
    require!(
        ctx.accounts.user.key() == admin_account.admin_pubkey,
        RewardError::UnauthorizedAdmin
    );
    admin_account.admin_pubkey = new_admin_pubkey;
    Ok(())

While this instruction can only be executed by the current admin, assigning an invalid or incorrect address results in an irreversible outcome. In scenarios where the admin role is transferred to an invalid or malicious account, critical operations requiring admin authorization would be permanently blocked or exploited.

To address this issue, consider implementing a two-step authority transfer functionality. This can be achieved by adding a function that designates a new authority candidate, ensuring that this candidate is not the zero address. Authority is only transferred once the new authority accepts it by sending a signed transaction.

It is also recommended to add a validation to ensure the new authority candidate is not the current admin.

SOLVED: The Wayru team solved this issue by adding two-step authority transfer process implementation ensuring the new candidate is not the current admin or the zero address.

The owner_claim_rewards and others_claim_rewards instructions allow the owner and host/manufacturer , respectively, to claim rewards by providing a reward amount as a parameter. The specified amount is transferred to the owner or host/manufacturer accordingly.

However, the program does not validate the provided amount in any of these instructions, leading into potential zero amount claims. The lack of validation allows users to provide a reward amount of zero when claiming rewards. If this occurs, the user calling the instruction will receive no rewards but the fields that track the last claim timestamp will be updated as if a valid claim occurred, preventing further reward claims during that day. This effectively causes the user to lose the opportunity to claim their rightful rewards for that day, as the state reflects a completed claim despite no rewards being distributed.

Although both operations require approval from the admin, as their partial signature is necessary, it is important to add validations to prevent the aforementioned scenario as a best practice.

other_claim_rewards.rs:

reward_entry.last_claimed_nonce = nonce; 
reward_entry.last_claimed_timestamp = current_timestamp;
    //verify if host or manufacturer
    if ctx.accounts.user.key() == nfnode_entry.host {
        nfnode_entry.host_last_claimed_timestamp = current_timestamp;
    } else if ctx.accounts.user.key() == nfnode_entry.manufacturer {
        nfnode_entry.manufacturer_last_claimed_timestamp = current_timestamp;
    } else {
        return Err(RewardError::UnauthorizedUser.into());
    }
    nfnode_entry.total_rewards_claimed += reward_amount;
    
    let authority_bump = ctx.bumps.token_storage_authority;
    let authority_seeds = &[&b"token_storage"[..], &[authority_bump]];
    let signer_seeds = &[&authority_seeds[..]];
    token::transfer(
        CpiContext::new_with_signer(
            ctx.accounts.token_program.to_account_info(),
            Transfer {
                from: ctx.accounts.token_storage_account.to_account_info(),
                to: ctx.accounts.user_token_account.to_account_info(),
                authority: ctx.accounts.token_storage_authority.to_account_info(),
            },
            signer_seeds
        ),
        reward_amount
    )?;

Similarly, the fund_token_storage instruction facilitates the funding of the token storage account by allowing users to provide a funding amount as a parameter. However the absence of validation for the provided funding amount allows users to perform funding operations with an amount of zero resulting in a zero-value transfer takes place, resulting in a redundant transaction with no functional impact.

lib.rs:

  pub fn fund_token_storage(ctx: Context<FundTokenStorage>, amount: u64) -> Result<()> {
        token::transfer(ctx.accounts.transfer_to_token_storage(), amount)?;
        Ok(())
    }

To address this issue, it is recommended to implement validations to ensure the reward amount in owner_claim_rewards and others_claim_rewards instructions is strictly greater than zero. Similarly, validate the funding amount in the fund_token_storage instruction to prevent zero-value operations.

SOLVED: The Wayru team solved this issue by adding:

• A check in owner_claim_rewards and others_claim_rewards instructions to ensure the provided reward_amount value is greater than 0.

• A validation in fund_token_storage to check the provided amount is greater than the zero value.

  
  

A user holding a previously minted NFT can initialize a new nfnode_entry using the init_nfnode instruction. To perform this action, the user must provide several accounts, including the nft_mint_address, user_nft_token_account, and others. This instruction initializes an nfnode_entry, which is a Program Derived Address (PDA) derived from a fixed seed and a variable component, specifically the nft_mint_address.

The instruction handler verifies that the user_nft_token_account is a token account whose associated mint matches the provided nft_mint_address and that its token amount is not zero. However, these validations are insufficient to ensure that the nft_mint_address corresponds to a valid NFT. Currently, the only validation for nft_mint_address is that it represents a Mint22, allowing any mint to pass as valid. Consequently, any user_nft_token_account associated with that mint and containing tokens would be accepted.

init_nfnode.rs:

 let user_nft_token_account_info = &ctx.accounts.user_nft_token_account;

    if user_nft_token_account_info.owner != &ctx.accounts.token_program_2022.key() {
        return err!(RewardError::InvalidNftMint);
    }

    let user_nft_token_account_data = user_nft_token_account_info.try_borrow_data()?;
    let user_nft_token_account = SplToken2022Account::try_deserialize(
        &mut &user_nft_token_account_data[..]
    )?;

    if user_nft_token_account.amount == 0 {
        return err!(RewardError::InsufficientNftBalance);
    }
   
    if user_nft_token_account.mint != ctx.accounts.nft_mint_address.key() {
        return err!(RewardError::InvalidNftMint);
    }
    let nfnode_entry = &mut ctx.accounts.nfnode_entry;
    nfnode_entry.host = ctx.accounts.host.key();

pub struct InitializeNfNode<'info> {
    #[account(mut)]
    pub user_admin: Signer<'info>,
    #[account(mut)]
    pub user: Signer<'info>,
    ///CHECK: only read account
    pub host: AccountInfo<'info>,
    ///CHECK: only read account
    pub manufacturer: AccountInfo<'info>,
    ///CHECK: only read account
    pub nft_mint_address: InterfaceAccount<'info, Mint2022>,
    /// CHECK: used to check nft ownership
    pub user_nft_token_account: AccountInfo<'info>,
    #[account(
        init,
        payer = user,
        space = 8 + std::mem::size_of::<NfNodeEntry>(),
        seeds = [b"nfnode_entry", nft_mint_address.key().as_ref()],
        bump
    )]
    pub nfnode_entry: Account<'info, NfNodeEntry>,

To address this issue, it is recommended to implement additional validation within the nfnode_entry initialization process to:

• Verify that the mint's total supply is exactly 1.

• Ensure that the mint's decimal precision is set to 0.

Note: Although the program does not perform transfers to or from the token accounts associated with these NFT mints, the fact that they are Mint22 accounts introduces the potential for unexpected extensions. Therefore, it is advisable to include a validation step that confirms the mint does not have any undesired extensions. This precaution would help mitigate unforeseen risks associated with improperly configured mints.

SOLVED: The Wayru team solved this issue by adding a validation that ensures that the total supply and decimals are those corresponding to the NFT. In addition, the AdminAccount now has a vector where your administrator can add and remove mint_authority addresses, which will correspond to the authorities that minted the allowed nft for new nfNode entries.

For the system initialization, the program’s upgrade authority is required to provide several accounts, including the mint_authority, which is added to the mint_authorities vector within the AdminAccount. The maximum amount of mint authority addresses allowed in the vector is ten. These represent the mint authorities responsible for minting the NFTs that will allow their owners to create a new NfNode entry.

However, the mint_authority account provided during initialization is not validated to ensure it corresponds to a valid and correct address. While this does not pose a significant security risk—since a trusted authority oversees the initialization process and can rectify errors by removing an incorrect mint_authority and adding the correct one—it is still recommended to implement a validation check to ensure that the provided address is at least not invalid.

init_system.rs:

pub fn initialize_system(ctx: Context<InitializeSystem>) -> Result<()> {
    let admin_account = &mut ctx.accounts.admin_account;
    admin_account.admin_pubkey = ctx.accounts.user.key();
    admin_account.paused = false;
    admin_account.valid_mint = ctx.accounts.token_mint.key();
    admin_account.mint_authorities.push(ctx.accounts.mint_authority.key());
    Ok(())
}

#[derive(Accounts)]
pub struct InitializeSystem<'info> {
    #[account(mut)]
    pub user: Signer<'info>,
    #[account(
        init,
        payer = user,
        space = 8 +
        std::mem::size_of::<AdminAccount>() +
        MAX_MINT_AUTHORITIES * std::mem::size_of::<Pubkey>(),
        seeds = [b"admin_account"],
        bump
    )]
    pub admin_account: Account<'info, AdminAccount>,
    ///CHECK: only read account
    pub mint_authority: AccountInfo<'info>,

Consider to add a validation to ensure the mint_authority provided is a trusted and valid address.

SOLVED: The Wayru team solved this issue by adding a check in the init_system instruction to ensure the mint_authority provided matches the upgrade authority of the program.

The administrator is granted the ability to manage the program’s operational state by pausing and unpausing it. This is achieved through the pause_program and unpause_program instructions, which respectively halt or resume the claiming of rewards.

Neither the pause_program nor the unpause_program instruction handlers validate whether the current program state already matches the intended state to be set. This oversight results in the program allowings the administrator to execute unnecessary state updates, such as pausing an already-paused program or unpausing an already-active one. These redundant actions incur transaction fees without resulting in any functional changes.

lib.rs:

   pub fn pause_program(ctx: Context<UpdateAdmin>) -> Result<()> {
        let admin_account = &mut ctx.accounts.admin_account;
        require!(
            ctx.accounts.user.key() == admin_account.admin_pubkey,
            RewardError::UnauthorizedAdmin
        );

        admin_account.paused = true;
        Ok(())
    }

    pub fn unpause_program(ctx: Context<UpdateAdmin>) -> Result<()> {
        let admin_account = &mut ctx.accounts.admin_account;
        require!(
            ctx.accounts.user.key() == admin_account.admin_pubkey,
            RewardError::UnauthorizedAdmin
        );

        admin_account.paused = false;

To enhance efficiency and prevent the mentioned redundant operations, it is recommended to implement the following measures:

• Add a check in the pause_program and unpause_program instruction handlers to verify whether the current state already matches the desired state.

• Return a clear error message when these redundant actions are attempted, informing the administrator that the state is already set to the requested value.

SOLVED: The Wayru team solved this issue by adding a check in the pause_program and unpause_program instruction handlers to ensure that the current status is not set again.