[message-buffer 15/x] - audit fixes (#817)

* fix(message-buffer): set payer as admin in initialize (TOB-PYTH-5)

* fix(message-buffer/delete_buffer): remove auth validation in delete_buffer & add test (TOB-PYTH-7)

* fix(message-buffer): fix non-security-related findings

use constant for seeds, cap max target size, add more tests

* refactor(message-buffer): use seed consts in initialize, remove unused errors

* refactor(message-buffer): address PR comments

refactor missed consts, make initialize more readable

Author: swimricky

pythnet/message_buffer/programs/message_buffer/src/instructions/create_buffer.rs

@@ -4,6 +4,7 @@ use {
         state::*,
         MessageBufferError,
         MESSAGE,
+        WHITELIST,
     },
     anchor_lang::{
         prelude::*,
@@ -23,6 +24,8 @@ pub fn create_buffer<'info>(
     base_account_key: Pubkey,
     target_size: u32,
 ) -> Result<()> {
+    require_keys_neq!(base_account_key, Pubkey::default());
+
     let buffer_account = ctx
         .remaining_accounts
         .first()
@@ -86,9 +89,9 @@ pub fn create_buffer<'info>(
 #[derive(Accounts)]
 pub struct CreateBuffer<'info> {
     #[account(
-    seeds = [b"message".as_ref(), b"whitelist".as_ref()],
-    bump = whitelist.bump,
-    has_one = admin,
+        seeds = [MESSAGE.as_bytes(), WHITELIST.as_bytes()],
+        bump = whitelist.bump,
+        has_one = admin,
     )]
     pub whitelist: Account<'info, Whitelist>,
 

pythnet/message_buffer/programs/message_buffer/src/instructions/delete_buffer.rs

@@ -1,24 +1,25 @@
 use {
-    crate::state::*,
+    crate::{
+        state::*,
+        MESSAGE,
+        WHITELIST,
+    },
     anchor_lang::prelude::*,
 };
 
 pub fn delete_buffer<'info>(
-    ctx: Context<'_, '_, '_, 'info, DeleteBuffer<'info>>,
-    allowed_program_auth: Pubkey,
+    _ctx: Context<'_, '_, '_, 'info, DeleteBuffer<'info>>,
+    _allowed_program_auth: Pubkey,
     _base_account_key: Pubkey,
 ) -> Result<()> {
-    ctx.accounts
-        .whitelist
-        .is_allowed_program_auth(&allowed_program_auth)?;
     Ok(())
 }
 
 #[derive(Accounts)]
 #[instruction(allowed_program_auth: Pubkey, base_account_key: Pubkey)]
 pub struct DeleteBuffer<'info> {
     #[account(
-        seeds = [b"message".as_ref(), b"whitelist".as_ref()],
+        seeds = [MESSAGE.as_bytes(), WHITELIST.as_bytes()],
         bump = whitelist.bump,
         has_one = admin,
     )]
@@ -29,10 +30,10 @@ pub struct DeleteBuffer<'info> {
     pub admin: Signer<'info>,
 
     #[account(
-    mut,
-    close = admin,
-    seeds = [allowed_program_auth.as_ref(), b"message".as_ref(), base_account_key.as_ref()],
-    bump = message_buffer.load()?.bump,
+        mut,
+        close = admin,
+        seeds = [allowed_program_auth.as_ref(), MESSAGE.as_bytes(), base_account_key.as_ref()],
+        bump = message_buffer.load()?.bump,
     )]
     pub message_buffer: AccountLoader<'info, MessageBuffer>,
 }

pythnet/message_buffer/programs/message_buffer/src/instructions/mod.rs

@@ -21,6 +21,7 @@ mod resize_buffer;
 // where authorized_program_pda is the where `allowed_program_auth`
 // is the whitelisted pubkey who authorized this call.
 pub const MESSAGE: &str = "message";
+pub const WHITELIST: &str = "whitelist";
 
 pub fn is_uninitialized_account(ai: &AccountInfo) -> bool {
     ai.data_is_empty() && ai.owner == &system_program::ID

pythnet/message_buffer/programs/message_buffer/src/instructions/put_all.rs

@@ -1,5 +1,8 @@
 use {
-    crate::state::*,
+    crate::{
+        state::*,
+        MESSAGE,
+    },
     anchor_lang::prelude::*,
 };
 
@@ -33,7 +36,7 @@ pub struct PutAll<'info> {
     pub whitelist_verifier: WhitelistVerifier<'info>,
     #[account(
         mut,
-        seeds = [whitelist_verifier.cpi_caller_auth.key().as_ref(), b"message".as_ref(), base_account_key.as_ref()],
+        seeds = [whitelist_verifier.cpi_caller_auth.key().as_ref(), MESSAGE.as_bytes(), base_account_key.as_ref()],
         bump = message_buffer.load()?.bump,
     )]
     pub message_buffer:     AccountLoader<'info, MessageBuffer>,

pythnet/message_buffer/programs/message_buffer/src/instructions/resize_buffer.rs

@@ -2,6 +2,8 @@ use {
     crate::{
         state::*,
         MessageBufferError,
+        MESSAGE,
+        WHITELIST,
     },
     anchor_lang::prelude::*,
 };
@@ -25,6 +27,13 @@ pub fn resize_buffer<'info>(
         minimum_size as usize,
         MessageBufferError::MessageBufferTooSmall
     );
+
+    require_gte!(
+        MessageBuffer::MAX_LEN as usize,
+        target_size as usize,
+        MessageBufferError::TargetSizeExceedsMaxLen
+    );
+
     Ok(())
 }
 
@@ -34,7 +43,7 @@ pub fn resize_buffer<'info>(
 )]
 pub struct ResizeBuffer<'info> {
     #[account(
-        seeds = [b"message".as_ref(), b"whitelist".as_ref()],
+        seeds = [MESSAGE.as_bytes(), WHITELIST.as_bytes()],
         bump = whitelist.bump,
         has_one = admin,
     )]
@@ -47,15 +56,16 @@ pub struct ResizeBuffer<'info> {
     pub system_program: Program<'info, System>,
 
     /// If decreasing, Anchor will automatically check
-    /// if target_size is too small and if so,then load() will fail.
+    /// if target_size is < MessageBuffer::INIT_SPACE + 8
+    /// and if so,then load() will fail.
     /// If increasing, Anchor also automatically checks if target_size delta
     /// exceeds MAX_PERMITTED_DATA_INCREASE
     #[account(
         mut,
         realloc = target_size as usize,
         realloc::zero = false,
         realloc::payer = admin,
-        seeds = [allowed_program_auth.as_ref(), b"message".as_ref(), base_account_key.as_ref()],
+        seeds = [allowed_program_auth.as_ref(), MESSAGE.as_bytes(), base_account_key.as_ref()],
         bump = message_buffer.load()?.bump,
     )]
     pub message_buffer: AccountLoader<'info, MessageBuffer>,

pythnet/message_buffer/programs/message_buffer/src/lib.rs

@@ -1,9 +1,12 @@
 pub mod instructions;
-mod macros;
 mod state;
 
 
 use {
+    crate::{
+        MESSAGE,
+        WHITELIST,
+    },
     anchor_lang::prelude::*,
     instructions::*,
     state::*,
@@ -16,13 +19,13 @@ pub mod message_buffer {
     use super::*;
 
 
-    /// Initializes the whitelist and sets it's admin to the provided pubkey
-    /// Once initialized, the authority must sign all further changes to the whitelist.
-    pub fn initialize(ctx: Context<Initialize>, admin: Pubkey) -> Result<()> {
-        require_keys_neq!(admin, Pubkey::default());
+    /// Initializes the whitelist and sets it's admin. Once initialized,
+    /// the admin must sign all further changes to the whitelist.
+    pub fn initialize(ctx: Context<Initialize>) -> Result<()> {
+        require_keys_neq!(ctx.accounts.admin.key(), Pubkey::default());
         let whitelist = &mut ctx.accounts.whitelist;
         whitelist.bump = *ctx.bumps.get("whitelist").unwrap();
-        whitelist.admin = admin;
+        whitelist.admin = ctx.accounts.admin.key();
         Ok(())
     }
 
@@ -141,12 +144,13 @@ pub mod message_buffer {
 
 #[derive(Accounts)]
 pub struct Initialize<'info> {
+    /// Admin that can update the whitelist and create/resize/delete buffers
     #[account(mut)]
-    pub payer:          Signer<'info>,
+    pub admin:          Signer<'info>,
     #[account(
         init,
-        payer = payer,
-        seeds = [b"message".as_ref(), b"whitelist".as_ref()],
+        payer = admin,
+        seeds = [MESSAGE.as_bytes(), WHITELIST.as_bytes()],
         bump,
         space = 8 + Whitelist::INIT_SPACE,
     )]
@@ -157,13 +161,10 @@ pub struct Initialize<'info> {
 
 #[derive(Accounts)]
 pub struct UpdateWhitelist<'info> {
-    #[account(mut)]
-    pub payer: Signer<'info>,
-
     pub admin:     Signer<'info>,
     #[account(
         mut,
-        seeds = [b"message".as_ref(), b"whitelist".as_ref()],
+        seeds = [MESSAGE.as_bytes(), WHITELIST.as_bytes()],
         bump = whitelist.bump,
         has_one = admin
     )]
@@ -175,32 +176,16 @@ pub struct UpdateWhitelist<'info> {
 pub enum MessageBufferError {
     #[msg("CPI Caller not allowed")]
     CallerNotAllowed,
-    #[msg("Whitelist already contains program")]
-    DuplicateAllowedProgram,
-    #[msg("Conversion Error")]
-    ConversionError,
-    #[msg("Serialization Error")]
-    SerializeError,
-    #[msg("Whitelist admin required on initialization")]
-    WhitelistAdminRequired,
     #[msg("Invalid allowed program")]
     InvalidAllowedProgram,
     #[msg("Maximum number of allowed programs exceeded")]
     MaximumAllowedProgramsExceeded,
-    #[msg("Invalid PDA")]
-    InvalidPDA,
-    #[msg("Update data exceeds current length")]
-    CurrentDataLengthExceeded,
     #[msg("Message Buffer not provided")]
     MessageBufferNotProvided,
     #[msg("Message Buffer target size is not sufficiently large")]
     MessageBufferTooSmall,
-    #[msg("Fund Bump not found")]
-    FundBumpNotFound,
-    #[msg("Reallocation failed")]
-    ReallocFailed,
     #[msg("Target size too large for reallocation/initialization. Max delta is 10240")]
     TargetSizeDeltaExceeded,
-    #[msg("MessageBuffer Uninitialized")]
-    MessageBufferUninitialized,
+    #[msg("Target size exceeds MessageBuffer::MAX_LEN")]
+    TargetSizeExceedsMaxLen,
 }