Skip to content

Commit 1f01ea2

Browse files
callensmcallensm
authored
address audit findings (#169)
1 parent 85434df commit 1f01ea2

15 files changed

+88
-22
lines changed

‎Cargo.lock

+1-1
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎programs/xnft/Cargo.toml

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
[package]
22
name = "xnft"
3-
version = "0.2.4"
3+
version = "0.2.5"
44
description = "Protocol for minting and managing xNFTs"
55
license = "GPL-3.0-only"
66
edition = "2021"

‎programs/xnft/src/instructions/create_app_xnft.rs

+9
Original file line numberDiff line numberDiff line change
@@ -152,11 +152,20 @@ pub fn create_app_xnft_handler(
152152
params: CreateXnftParams,
153153
) -> Result<()> {
154154
// Check the length of the metadata uri provided.
155+
//
156+
// The argued name does not need to be validated since the maximum
157+
// length is the same as the max seed length, meaning the instruction
158+
// will already fail if the name exceeds that.
155159
require!(
156160
params.uri.len() <= MAX_URI_LENGTH,
157161
CustomError::UriExceedsMaxLength,
158162
);
159163

164+
// Check that if a supply was provided it is greater than 0.
165+
if let Some(s) = params.supply {
166+
require_gt!(s, 0);
167+
}
168+
160169
// Initialize and populate the new xNFT program account data.
161170
let xnft = &mut ctx.accounts.xnft;
162171
***xnft = Xnft::try_new(

‎programs/xnft/src/instructions/create_install.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ pub struct CreateInstall<'info> {
3939
////////////////////////////////////////////////////////////////////////////
4040
#[account(
4141
init,
42-
payer = authority,
42+
payer = target,
4343
space = Install::LEN,
4444
seeds = [
4545
"install".as_bytes(),
@@ -51,8 +51,8 @@ pub struct CreateInstall<'info> {
5151
pub install: Account<'info, Install>,
5252

5353
#[account(mut)]
54-
pub authority: Signer<'info>,
5554
pub target: Signer<'info>,
55+
pub authority: Signer<'info>,
5656

5757
pub system_program: Program<'info, System>,
5858
}

‎programs/xnft/src/instructions/create_permissioned_install.rs

+7
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ pub struct CreatePermissionedInstall<'info> {
2727
has_one = install_vault,
2828
constraint = xnft.kind == Kind::App @ CustomError::MustBeApp,
2929
constraint = !xnft.suspended @ CustomError::SuspendedInstallation,
30+
constraint = xnft.install_authority == Some(*install_authority.key) @ CustomError::InstallAuthorityMismatch,
3031
)]
3132
pub xnft: Account<'info, Xnft>,
3233

@@ -51,6 +52,8 @@ pub struct CreatePermissionedInstall<'info> {
5152
pub install: Account<'info, Install>,
5253

5354
#[account(
55+
mut,
56+
close = install_authority,
5457
seeds = [
5558
"access".as_bytes(),
5659
authority.key().as_ref(),
@@ -62,6 +65,10 @@ pub struct CreatePermissionedInstall<'info> {
6265
)]
6366
pub access: Account<'info, Access>,
6467

68+
/// CHECK: account matching constraint on the `xnft` account.
69+
#[account(mut)]
70+
pub install_authority: UncheckedAccount<'info>,
71+
6572
#[account(mut)]
6673
pub authority: Signer<'info>,
6774

‎programs/xnft/src/instructions/create_review.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ use anchor_spl::token::TokenAccount;
1818

1919
use crate::events::ReviewCreated;
2020
use crate::state::{Install, Kind, Review, Xnft};
21-
use crate::{CustomError, MAX_RATING};
21+
use crate::{CustomError, MAX_RATING, MIN_RATING};
2222

2323
#[derive(Accounts)]
2424
#[instruction(uri: String)]
@@ -69,7 +69,7 @@ pub fn create_review_handler(ctx: Context<CreateReview>, uri: String, rating: u8
6969
let xnft = &mut ctx.accounts.xnft;
7070
let review = &mut ctx.accounts.review;
7171

72-
if rating > MAX_RATING {
72+
if !(MIN_RATING..=MAX_RATING).contains(&rating) {
7373
return Err(error!(CustomError::RatingOutOfBounds));
7474
}
7575

‎programs/xnft/src/instructions/set_curator_verification.rs

+2
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,8 @@ pub fn set_curator_verification_handler(
3535
) -> Result<()> {
3636
if let Some(curator) = &mut ctx.accounts.xnft.curator {
3737
curator.verified = value;
38+
} else {
39+
return Err(error!(CustomError::CuratorMismatch));
3840
}
3941
Ok(())
4042
}

‎programs/xnft/src/instructions/update_xnft.rs

+5-3
Original file line numberDiff line numberDiff line change
@@ -110,7 +110,7 @@ pub fn update_xnft_handler(ctx: Context<UpdateXnft>, updates: UpdateParams) -> R
110110
ctx.accounts
111111
.update_metadata_accounts_ctx()
112112
.with_signer(&[&ctx.accounts.xnft.as_seeds()]),
113-
Some(md.update_authority),
113+
None,
114114
Some(DataV2 {
115115
name: updates.name.unwrap_or_else(|| md.data.name.clone()),
116116
symbol: md.data.symbol.clone(),
@@ -120,8 +120,8 @@ pub fn update_xnft_handler(ctx: Context<UpdateXnft>, updates: UpdateParams) -> R
120120
collection: md.collection.clone(),
121121
uses: md.uses.clone(),
122122
}),
123-
Some(true),
124-
Some(md.is_mutable),
123+
None,
124+
None,
125125
)?;
126126
}
127127
}
@@ -155,6 +155,8 @@ pub fn update_xnft_handler(ctx: Context<UpdateXnft>, updates: UpdateParams) -> R
155155
{
156156
return Err(error!(CustomError::SupplyReduction));
157157
}
158+
159+
require_gt!(new_supply, 0);
158160
xnft.supply = Some(new_supply);
159161
}
160162
None => {

‎programs/xnft/src/lib.rs

+2
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,8 @@ security_txt! {
4141

4242
#[constant]
4343
pub const MAX_RATING: u8 = 5;
44+
#[constant]
45+
pub const MIN_RATING: u8 = 1;
4446

4547
#[program]
4648
pub mod xnft {

‎programs/xnft/src/state/xnft.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -135,7 +135,7 @@ impl Xnft {
135135

136136
pub fn verify_supply(&self) -> anchor_lang::Result<()> {
137137
if let Some(supply) = self.supply {
138-
if supply > 0 && self.total_installs >= supply {
138+
if supply == 0 || (supply > 0 && self.total_installs >= supply) {
139139
return Err(error!(CustomError::InstallExceedsSupply));
140140
}
141141
}

‎tests/private.spec.ts

+4
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,10 @@ describe("Private xNFTs", () => {
6363
});
6464

6565
describe("access can be revoked by the install authority", () => {
66+
before(async () => {
67+
await c.grantAccess(privateXnft, grantee.publicKey);
68+
});
69+
6670
it("using the revoke_access instruction", async () => {
6771
await c.revokeAccess(privateXnft, grantee.publicKey);
6872
});

‎tests/xnft.spec.ts

+10
Original file line numberDiff line numberDiff line change
@@ -235,6 +235,16 @@ describe("A standard xNFT", () => {
235235
}
236236
});
237237

238+
it("unless the provided rating is less than 1", async () => {
239+
try {
240+
await c.review("https://google.com", 0, xnft, masterToken);
241+
assert.ok(false);
242+
} catch (err) {
243+
const e = err as anchor.AnchorError;
244+
assert.strictEqual(e.error.errorCode.code, "RatingOutOfBounds");
245+
}
246+
});
247+
238248
it("when appropriate accounts and arguments", async () => {
239249
[review] = deriveReviewAddress(xnft, author.publicKey);
240250
await c.review("https://google.com", 4, xnft, masterToken);

‎typescript/package.json

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"name": "@coral-xyz/xnft",
3-
"version": "0.2.58",
3+
"version": "0.2.59",
44
"license": "GPL-3.0-only",
55
"description": "Node.js client for the xNFT protocol",
66
"repository": {

‎typescript/src/instructions.ts

+13-3
Original file line numberDiff line numberDiff line change
@@ -140,9 +140,19 @@ export async function createCreateInstallInstruction(
140140
installVault: PublicKey,
141141
permissioned?: boolean
142142
): Promise<TransactionInstruction> {
143-
return permissioned
144-
? await program.methods.createPermissionedInstall().accounts({ xnft, installVault }).instruction()
145-
: await program.methods.createInstall().accounts({ xnft, installVault }).instruction();
143+
if (permissioned) {
144+
const data = await program.account.xnft.fetch(xnft);
145+
146+
if (!data.installAuthority) {
147+
throw new Error("attempting a permissioned installation when no install authority is set");
148+
}
149+
150+
return program.methods
151+
.createPermissionedInstall()
152+
.accounts({ xnft, installVault, installAuthority: data.installAuthority })
153+
.instruction();
154+
}
155+
return program.methods.createInstall().accounts({ xnft, installVault }).instruction();
146156
}
147157

148158
/**

‎typescript/src/xnft.ts

+28-8
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,16 @@
11
export type Xnft = {
2-
version: "0.2.4";
2+
version: "0.2.5";
33
name: "xnft";
44
constants: [
55
{
66
name: "MAX_RATING";
77
type: "u8";
88
value: "5";
9+
},
10+
{
11+
name: "MIN_RATING";
12+
type: "u8";
13+
value: "1";
914
}
1015
];
1116
instructions: [
@@ -253,12 +258,12 @@ export type Xnft = {
253258
};
254259
},
255260
{
256-
name: "authority";
261+
name: "target";
257262
isMut: true;
258263
isSigner: true;
259264
},
260265
{
261-
name: "target";
266+
name: "authority";
262267
isMut: false;
263268
isSigner: true;
264269
},
@@ -315,7 +320,7 @@ export type Xnft = {
315320
},
316321
{
317322
name: "access";
318-
isMut: false;
323+
isMut: true;
319324
isSigner: false;
320325
pda: {
321326
seeds: [
@@ -339,6 +344,11 @@ export type Xnft = {
339344
};
340345
relations: ["xnft"];
341346
},
347+
{
348+
name: "installAuthority";
349+
isMut: true;
350+
isSigner: false;
351+
},
342352
{
343353
name: "authority";
344354
isMut: true;
@@ -1433,14 +1443,19 @@ export type Xnft = {
14331443
};
14341444

14351445
export const IDL: Xnft = {
1436-
version: "0.2.4",
1446+
version: "0.2.5",
14371447
name: "xnft",
14381448
constants: [
14391449
{
14401450
name: "MAX_RATING",
14411451
type: "u8",
14421452
value: "5",
14431453
},
1454+
{
1455+
name: "MIN_RATING",
1456+
type: "u8",
1457+
value: "1",
1458+
},
14441459
],
14451460
instructions: [
14461461
{
@@ -1687,12 +1702,12 @@ export const IDL: Xnft = {
16871702
},
16881703
},
16891704
{
1690-
name: "authority",
1705+
name: "target",
16911706
isMut: true,
16921707
isSigner: true,
16931708
},
16941709
{
1695-
name: "target",
1710+
name: "authority",
16961711
isMut: false,
16971712
isSigner: true,
16981713
},
@@ -1749,7 +1764,7 @@ export const IDL: Xnft = {
17491764
},
17501765
{
17511766
name: "access",
1752-
isMut: false,
1767+
isMut: true,
17531768
isSigner: false,
17541769
pda: {
17551770
seeds: [
@@ -1773,6 +1788,11 @@ export const IDL: Xnft = {
17731788
},
17741789
relations: ["xnft"],
17751790
},
1791+
{
1792+
name: "installAuthority",
1793+
isMut: true,
1794+
isSigner: false,
1795+
},
17761796
{
17771797
name: "authority",
17781798
isMut: true,

0 commit comments

Comments
 (0)