Permissions (#289)

* Permissions

* More tests

* Generic function for all authorities

* Switch to one match

* Mapping is not a signer

* Is authorized uses key

* Add rent logic

* Comment

* Fix error type

Author: guibescos

program/rust/src/c_oracle_header.rs

@@ -9,6 +9,8 @@ use bytemuck::{
 };
 use solana_program::pubkey::Pubkey;
 use std::mem::size_of;
+
+use crate::instruction::OracleCommand;
 //bindings.rs is generated by build.rs to include
 //things defined in bindings.h
 include!("../bindings.rs");
@@ -89,6 +91,16 @@ pub struct PermissionAccount {
     pub security_authority:      Pubkey,
 }
 
+impl PermissionAccount {
+    pub fn is_authorized(&self, key: &Pubkey, command: OracleCommand) -> bool {
+        #[allow(clippy::match_like_matches_macro)]
+        match (*key, command) {
+            (pubkey, OracleCommand::InitMapping) if pubkey == self.master_authority => true,
+            _ => false,
+        }
+    }
+}
+
 #[repr(C)]
 #[derive(Copy, Clone, Pod, Zeroable)]
 pub struct PriceAccount {

program/rust/src/error.rs

@@ -44,6 +44,10 @@ pub enum OracleError {
     InvalidAccountHeader           = 616,
     #[error("InvalidNumberOfAccounts")]
     InvalidNumberOfAccounts        = 617,
+    #[error("InvalidReadableAccount")]
+    InvalidReadableAccount         = 618,
+    #[error("PermissionViolation")]
+    PermissionViolation            = 619,
 }
 
 impl From<OracleError> for ProgramError {

program/rust/src/rust_oracle.rs

@@ -44,6 +44,7 @@ use crate::utils::{
     check_is_upgrade_authority_for_program,
     check_valid_funding_account,
     check_valid_signable_account,
+    check_valid_signable_account_or_permissioned_funding_account,
     check_valid_writable_account,
     get_rent,
     is_component_update,
@@ -146,16 +147,24 @@ pub fn init_mapping(
     accounts: &[AccountInfo],
     instruction_data: &[u8],
 ) -> ProgramResult {
-    let [funding_account, fresh_mapping_account] = match accounts {
-        [x, y] => Ok([x, y]),
+    let (funding_account, fresh_mapping_account, permissions_account_option) = match accounts {
+        [x, y] => Ok((x, y, None)),
+        [x, y, p] => Ok((x, y, Some(p))),
         _ => Err(OracleError::InvalidNumberOfAccounts),
     }?;
 
+    let hdr = load::<CommandHeader>(instruction_data)?;
+
     check_valid_funding_account(funding_account)?;
-    check_valid_signable_account(program_id, fresh_mapping_account)?;
+    check_valid_signable_account_or_permissioned_funding_account(
+        program_id,
+        fresh_mapping_account,
+        funding_account,
+        permissions_account_option,
+        hdr,
+    )?;
 
     // Initialize by setting to zero again (just in case) and populating the account header
-    let hdr = load::<CommandHeader>(instruction_data)?;
     initialize_pyth_account_checked::<MappingAccount>(fresh_mapping_account, hdr.version)?;
 
     Ok(())

program/rust/src/tests/test_init_mapping.rs

@@ -1,10 +1,14 @@
 use crate::c_oracle_header::{
     MappingAccount,
+    PermissionAccount,
     PC_ACCTYPE_MAPPING,
     PC_MAGIC,
     PC_VERSION,
 };
-use crate::deserialize::load_account_as;
+use crate::deserialize::{
+    initialize_pyth_account_checked,
+    load_account_as,
+};
 use crate::error::OracleError;
 use crate::instruction::{
     CommandHeader,
@@ -29,6 +33,9 @@ fn test_init_mapping() {
     let mut funding_setup = AccountSetup::new_funding();
     let mut funding_account = funding_setup.to_account_info();
 
+    let mut attacker_setup = AccountSetup::new_funding();
+    let attacker_account = attacker_setup.to_account_info();
+
     let mut mapping_setup = AccountSetup::new::<MappingAccount>(&program_id);
     let mut mapping_account = mapping_setup.to_account_info();
 
@@ -144,4 +151,84 @@ fn test_init_mapping() {
         instruction_data
     )
     .is_ok());
+
+    clear_account(&mapping_account).unwrap();
+    mapping_account.is_signer = false;
+
+    let mut permissions_setup = AccountSetup::new_permission(&program_id);
+    let permissions_account = permissions_setup.to_account_info();
+
+    // Permissions account is unitialized
+    assert_eq!(
+        process_instruction(
+            &program_id,
+            &[
+                funding_account.clone(),
+                mapping_account.clone(),
+                permissions_account.clone()
+            ],
+            instruction_data
+        ),
+        Err(OracleError::InvalidAccountHeader.into())
+    );
+
+    {
+        let mut permissions_account_data =
+            initialize_pyth_account_checked::<PermissionAccount>(&permissions_account, PC_VERSION)
+                .unwrap();
+        permissions_account_data.master_authority = *funding_account.key;
+    }
+
+    // Attacker account tries to sign transaction instead of funding account
+    assert_eq!(
+        process_instruction(
+            &program_id,
+            &[
+                attacker_account.clone(),
+                mapping_account.clone(),
+                permissions_account.clone()
+            ],
+            instruction_data
+        ),
+        Err(OracleError::PermissionViolation.into())
+    );
+
+    // Attacker tries to impersonate permissions account
+    let mut impersonating_permission_setup = AccountSetup::new::<PermissionAccount>(&program_id);
+    let impersonating_permission_account = impersonating_permission_setup.to_account_info();
+
+    {
+        let mut impersonating_permission_account_data =
+            initialize_pyth_account_checked::<PermissionAccount>(
+                &impersonating_permission_account,
+                PC_VERSION,
+            )
+            .unwrap();
+        impersonating_permission_account_data.master_authority = *attacker_account.key;
+    }
+
+    assert_eq!(
+        process_instruction(
+            &program_id,
+            &[
+                attacker_account.clone(),
+                mapping_account.clone(),
+                impersonating_permission_account.clone()
+            ],
+            instruction_data
+        ),
+        Err(OracleError::InvalidPda.into())
+    );
+
+    // Right authority, initialized permissions account
+    assert!(process_instruction(
+        &program_id,
+        &[
+            funding_account.clone(),
+            mapping_account.clone(),
+            permissions_account.clone()
+        ],
+        instruction_data
+    )
+    .is_ok());
 }

program/rust/src/tests/test_utils.rs

@@ -1,6 +1,8 @@
 use crate::c_oracle_header::{
+    PermissionAccount,
     PythAccount,
     PC_VERSION,
+    PERMISSIONS_SEED,
 };
 use crate::error::OracleError;
 use crate::instruction::{
@@ -75,6 +77,21 @@ impl AccountSetup {
         };
     }
 
+    pub fn new_permission(owner: &Pubkey) -> Self {
+        let (key, _bump) = Pubkey::find_program_address(&[PERMISSIONS_SEED.as_bytes()], owner);
+        let owner = owner.clone();
+        let balance = Rent::minimum_balance(&Rent::default(), PermissionAccount::MINIMUM_SIZE);
+        let size = PermissionAccount::MINIMUM_SIZE;
+        let data = [0; UPPER_BOUND_OF_ALL_ACCOUNT_SIZES];
+        return AccountSetup {
+            key,
+            owner,
+            balance,
+            size,
+            data,
+        };
+    }
+
     pub fn new_clock() -> Self {
         let key = clock::Clock::id();
         let owner = sysvar::id();

program/rust/src/utils.rs

@@ -1,9 +1,15 @@
 use crate::c_oracle_header::{
     AccountHeader,
+    PermissionAccount,
     MAX_NUM_DECIMALS,
+    PERMISSIONS_SEED,
+};
+use crate::deserialize::{
+    load_account_as,
+    load_checked,
 };
-use crate::deserialize::load_account_as;
 use crate::instruction::{
+    CommandHeader,
     OracleCommand,
     UpdPriceArgs,
 };
@@ -76,6 +82,34 @@ pub fn check_valid_signable_account(
     )
 }
 
+/// Check that `account` is a valid signable pyth account or
+/// that `funding_account` is a signer and is permissioned by the `permission_account`
+pub fn check_valid_signable_account_or_permissioned_funding_account(
+    program_id: &Pubkey,
+    account: &AccountInfo,
+    funding_account: &AccountInfo,
+    permissions_account_option: Option<&AccountInfo>,
+    cmd_hdr: &CommandHeader,
+) -> Result<(), ProgramError> {
+    if let Some(permissions_account) = permissions_account_option {
+        check_valid_permissions_account(program_id, permissions_account)?;
+        let permissions_account_data =
+            load_checked::<PermissionAccount>(permissions_account, cmd_hdr.version)?;
+        check_valid_funding_account(funding_account)?;
+        pyth_assert(
+            permissions_account_data.is_authorized(
+                funding_account.key,
+                OracleCommand::from_i32(cmd_hdr.command)
+                    .ok_or(OracleError::UnrecognizedInstruction)?,
+            ),
+            OracleError::PermissionViolation.into(),
+        )?;
+        check_valid_writable_account(program_id, account)
+    } else {
+        check_valid_signable_account(program_id, account)
+    }
+}
+
 /// Returns `true` if the `account` is fresh, i.e., its data can be overwritten.
 /// Use this check to prevent accidentally overwriting accounts whose data is already populated.
 pub fn valid_fresh_account(account: &AccountInfo) -> bool {
@@ -141,6 +175,40 @@ pub fn check_valid_writable_account(
     )
 }
 
+
+fn valid_readable_account(
+    program_id: &Pubkey,
+    account: &AccountInfo,
+) -> Result<bool, ProgramError> {
+    Ok(
+        account.owner == program_id
+            && get_rent()?.is_exempt(account.lamports(), account.data_len()),
+    )
+}
+
+pub fn check_valid_readable_account(
+    program_id: &Pubkey,
+    account: &AccountInfo,
+) -> Result<(), ProgramError> {
+    pyth_assert(
+        valid_readable_account(program_id, account)?,
+        OracleError::InvalidReadableAccount.into(),
+    )
+}
+
+pub fn check_valid_permissions_account(
+    program_id: &Pubkey,
+    account: &AccountInfo,
+) -> Result<(), ProgramError> {
+    check_valid_readable_account(program_id, account)?;
+    let (permission_pda_address, _) =
+        Pubkey::find_program_address(&[PERMISSIONS_SEED.as_bytes()], program_id);
+    pyth_assert(
+        permission_pda_address == *account.key,
+        OracleError::InvalidPda.into(),
+    )
+}
+
 /// Checks whether this instruction is trying to update an individual publisher's price (`true`) or
 /// is only trying to refresh the aggregate (`false`)
 pub fn is_component_update(cmd_args: &UpdPriceArgs) -> Result<bool, OracleError> {