address audit findings (#169)

programs/xnft/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "xnft"
-version = "0.2.4"
+version = "0.2.5"
 description = "Protocol for minting and managing xNFTs"
 license = "GPL-3.0-only"
 edition = "2021"

programs/xnft/src/instructions/create_app_xnft.rs

@@ -152,11 +152,20 @@ pub fn create_app_xnft_handler(
     params: CreateXnftParams,
 ) -> Result<()> {
     // Check the length of the metadata uri provided.
+    //
+    // The argued name does not need to be validated since the maximum
+    // length is the same as the max seed length, meaning the instruction
+    // will already fail if the name exceeds that.
     require!(
         params.uri.len() <= MAX_URI_LENGTH,
         CustomError::UriExceedsMaxLength,
     );
 
+    // Check that if a supply was provided it is greater than 0.
+    if let Some(s) = params.supply {
+        require_gt!(s, 0);
+    }
+
     // Initialize and populate the new xNFT program account data.
     let xnft = &mut ctx.accounts.xnft;
     ***xnft = Xnft::try_new(

programs/xnft/src/instructions/create_install.rs

@@ -39,7 +39,7 @@ pub struct CreateInstall<'info> {
     ////////////////////////////////////////////////////////////////////////////
     #[account(
         init,
-        payer = authority,
+        payer = target,
         space = Install::LEN,
         seeds = [
             "install".as_bytes(),
@@ -51,8 +51,8 @@ pub struct CreateInstall<'info> {
     pub install: Account<'info, Install>,
 
     #[account(mut)]
-    pub authority: Signer<'info>,
     pub target: Signer<'info>,
+    pub authority: Signer<'info>,
 
     pub system_program: Program<'info, System>,
 }

programs/xnft/src/instructions/create_permissioned_install.rs

@@ -27,6 +27,7 @@ pub struct CreatePermissionedInstall<'info> {
         has_one = install_vault,
         constraint = xnft.kind == Kind::App @ CustomError::MustBeApp,
         constraint = !xnft.suspended @ CustomError::SuspendedInstallation,
+        constraint = xnft.install_authority == Some(*install_authority.key) @ CustomError::InstallAuthorityMismatch,
     )]
     pub xnft: Account<'info, Xnft>,
 
@@ -51,6 +52,8 @@ pub struct CreatePermissionedInstall<'info> {
     pub install: Account<'info, Install>,
 
     #[account(
+        mut,
+        close = install_authority,
         seeds = [
             "access".as_bytes(),
             authority.key().as_ref(),
@@ -62,6 +65,10 @@ pub struct CreatePermissionedInstall<'info> {
     )]
     pub access: Account<'info, Access>,
 
+    /// CHECK: account matching constraint on the `xnft` account.
+    #[account(mut)]
+    pub install_authority: UncheckedAccount<'info>,
+
     #[account(mut)]
     pub authority: Signer<'info>,
 

programs/xnft/src/instructions/create_review.rs

@@ -18,7 +18,7 @@ use anchor_spl::token::TokenAccount;
 
 use crate::events::ReviewCreated;
 use crate::state::{Install, Kind, Review, Xnft};
-use crate::{CustomError, MAX_RATING};
+use crate::{CustomError, MAX_RATING, MIN_RATING};
 
 #[derive(Accounts)]
 #[instruction(uri: String)]
@@ -69,7 +69,7 @@ pub fn create_review_handler(ctx: Context<CreateReview>, uri: String, rating: u8
     let xnft = &mut ctx.accounts.xnft;
     let review = &mut ctx.accounts.review;
 
-    if rating > MAX_RATING {
+    if !(MIN_RATING..=MAX_RATING).contains(&rating) {
         return Err(error!(CustomError::RatingOutOfBounds));
     }
 

programs/xnft/src/instructions/set_curator_verification.rs

@@ -35,6 +35,8 @@ pub fn set_curator_verification_handler(
 ) -> Result<()> {
     if let Some(curator) = &mut ctx.accounts.xnft.curator {
         curator.verified = value;
+    } else {
+        return Err(error!(CustomError::CuratorMismatch));
     }
     Ok(())
 }

programs/xnft/src/instructions/update_xnft.rs

@@ -110,7 +110,7 @@ pub fn update_xnft_handler(ctx: Context<UpdateXnft>, updates: UpdateParams) -> R
                 ctx.accounts
                     .update_metadata_accounts_ctx()
                     .with_signer(&[&ctx.accounts.xnft.as_seeds()]),
-                Some(md.update_authority),
+                None,
                 Some(DataV2 {
                     name: updates.name.unwrap_or_else(|| md.data.name.clone()),
                     symbol: md.data.symbol.clone(),
@@ -120,8 +120,8 @@ pub fn update_xnft_handler(ctx: Context<UpdateXnft>, updates: UpdateParams) -> R
                     collection: md.collection.clone(),
                     uses: md.uses.clone(),
                 }),
-                Some(true),
-                Some(md.is_mutable),
+                None,
+                None,
             )?;
         }
     }
@@ -155,6 +155,8 @@ pub fn update_xnft_handler(ctx: Context<UpdateXnft>, updates: UpdateParams) -> R
             {
                 return Err(error!(CustomError::SupplyReduction));
             }
+
+            require_gt!(new_supply, 0);
             xnft.supply = Some(new_supply);
         }
         None => {

programs/xnft/src/lib.rs

@@ -41,6 +41,8 @@ security_txt! {
 
 #[constant]
 pub const MAX_RATING: u8 = 5;
+#[constant]
+pub const MIN_RATING: u8 = 1;
 
 #[program]
 pub mod xnft {

programs/xnft/src/state/xnft.rs

@@ -135,7 +135,7 @@ impl Xnft {
 
     pub fn verify_supply(&self) -> anchor_lang::Result<()> {
         if let Some(supply) = self.supply {
-            if supply > 0 && self.total_installs >= supply {
+            if supply == 0 || (supply > 0 && self.total_installs >= supply) {
                 return Err(error!(CustomError::InstallExceedsSupply));
             }
         }

tests/private.spec.ts

@@ -63,6 +63,10 @@ describe("Private xNFTs", () => {
   });
 
   describe("access can be revoked by the install authority", () => {
+    before(async () => {
+      await c.grantAccess(privateXnft, grantee.publicKey);
+    });
+
     it("using the revoke_access instruction", async () => {
       await c.revokeAccess(privateXnft, grantee.publicKey);
     });

tests/xnft.spec.ts

@@ -235,6 +235,16 @@ describe("A standard xNFT", () => {
       }
     });
 
+    it("unless the provided rating is less than 1", async () => {
+      try {
+        await c.review("https://google.com", 0, xnft, masterToken);
+        assert.ok(false);
+      } catch (err) {
+        const e = err as anchor.AnchorError;
+        assert.strictEqual(e.error.errorCode.code, "RatingOutOfBounds");
+      }
+    });
+
     it("when appropriate accounts and arguments", async () => {
       [review] = deriveReviewAddress(xnft, author.publicKey);
       await c.review("https://google.com", 4, xnft, masterToken);

typescript/package.json

@@ -1,6 +1,6 @@
 {
   "name": "@coral-xyz/xnft",
-  "version": "0.2.58",
+  "version": "0.2.59",
   "license": "GPL-3.0-only",
   "description": "Node.js client for the xNFT protocol",
   "repository": {

typescript/src/instructions.ts

@@ -140,9 +140,19 @@ export async function createCreateInstallInstruction(
   installVault: PublicKey,
   permissioned?: boolean
 ): Promise<TransactionInstruction> {
-  return permissioned
-    ? await program.methods.createPermissionedInstall().accounts({ xnft, installVault }).instruction()
-    : await program.methods.createInstall().accounts({ xnft, installVault }).instruction();
+  if (permissioned) {
+    const data = await program.account.xnft.fetch(xnft);
+
+    if (!data.installAuthority) {
+      throw new Error("attempting a permissioned installation when no install authority is set");
+    }
+
+    return program.methods
+      .createPermissionedInstall()
+      .accounts({ xnft, installVault, installAuthority: data.installAuthority })
+      .instruction();
+  }
+  return program.methods.createInstall().accounts({ xnft, installVault }).instruction();
 }
 
 /**

typescript/src/xnft.ts

@@ -1,11 +1,16 @@
 export type Xnft = {
-  version: "0.2.4";
+  version: "0.2.5";
   name: "xnft";
   constants: [
     {
       name: "MAX_RATING";
       type: "u8";
       value: "5";
+    },
+    {
+      name: "MIN_RATING";
+      type: "u8";
+      value: "1";
     }
   ];
   instructions: [
@@ -253,12 +258,12 @@ export type Xnft = {
           };
         },
         {
-          name: "authority";
+          name: "target";
           isMut: true;
           isSigner: true;
         },
         {
-          name: "target";
+          name: "authority";
           isMut: false;
           isSigner: true;
         },
@@ -315,7 +320,7 @@ export type Xnft = {
         },
         {
           name: "access";
-          isMut: false;
+          isMut: true;
           isSigner: false;
           pda: {
             seeds: [
@@ -339,6 +344,11 @@ export type Xnft = {
           };
           relations: ["xnft"];
         },
+        {
+          name: "installAuthority";
+          isMut: true;
+          isSigner: false;
+        },
         {
           name: "authority";
           isMut: true;
@@ -1433,14 +1443,19 @@ export type Xnft = {
 };
 
 export const IDL: Xnft = {
-  version: "0.2.4",
+  version: "0.2.5",
   name: "xnft",
   constants: [
     {
       name: "MAX_RATING",
       type: "u8",
       value: "5",
     },
+    {
+      name: "MIN_RATING",
+      type: "u8",
+      value: "1",
+    },
   ],
   instructions: [
     {
@@ -1687,12 +1702,12 @@ export const IDL: Xnft = {
           },
         },
         {
-          name: "authority",
+          name: "target",
           isMut: true,
           isSigner: true,
         },
         {
-          name: "target",
+          name: "authority",
           isMut: false,
           isSigner: true,
         },
@@ -1749,7 +1764,7 @@ export const IDL: Xnft = {
         },
         {
           name: "access",
-          isMut: false,
+          isMut: true,
           isSigner: false,
           pda: {
             seeds: [
@@ -1773,6 +1788,11 @@ export const IDL: Xnft = {
           },
           relations: ["xnft"],
         },
+        {
+          name: "installAuthority",
+          isMut: true,
+          isSigner: false,
+        },
         {
           name: "authority",
           isMut: true,