Skip to content

Commit 660c76b

Browse files
committed
fix lint-6 tests and add readme
1 parent d0f89cc commit 660c76b

File tree

5 files changed

+117
-40
lines changed

5 files changed

+117
-40
lines changed
Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
# duplicate_mutable_accounts
2+
3+
**What it does:** Checks to make sure there is a key check on identical Anchor accounts.
4+
The key check serves to make sure that two identical accounts do not have the same key,
5+
ie, they are unique. An Anchor account (`Account<'info, T>`) is identical to another if
6+
the generic parameter `T` is the same type for each account.
7+
8+
**Why is this bad?** If a program contains two identical, mutable Anchor accounts, and
9+
performs some operation on those accounts, then a user could pass in the same account
10+
twice. Then any previous operations may be overwritten by the last operation, which may
11+
not be what the program wanted if it expected different accounts.
12+
13+
**Known problems:** If a program is not using the anchor `#[account]` macro constraints,
14+
and is instead using checks in the function bodies, and the program uses boolean operator
15+
&& or || to link constraints in a single if statement, the lint will flag this as a false
16+
positive since the lint only catches statements with `==` or `!=`.
17+
18+
Another issue is if a program uses an if statement such as `a.key() == b.key()` and then
19+
continues to modify the accounts, then this will not be caught. The reason is because the
20+
lint regards expressions with `==` as a secure check, since it assumes the program will
21+
then return an error (see the secure example). However, it does not explicitly check that
22+
an error is returned.
23+
24+
In general, this lint will catch all vulnerabilities if the anchor macro constraints are
25+
used (see the recommended example). It is not as robust if alternative methods are utilized.
26+
Thus it is encouraged to use the anchor `#[account]` macro constraints.
27+
28+
**Example:**
29+
30+
```rust
31+
#[derive(Accounts)]
32+
pub struct Update<'info> {
33+
user_a: Account<'info, User>,
34+
user_b: Account<'info, User>,
35+
}
36+
```
37+
38+
Use instead:
39+
40+
```rust
41+
#[derive(Accounts)]
42+
pub struct Update<'info> {
43+
#[account(constraint = user_a.key() != user_b.key())]
44+
user_a: Account<'info, User>,
45+
user_b: Account<'info, User>,
46+
}
47+
```

lints/duplicate-mutable-accounts/src/lib.rs

Lines changed: 35 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -27,24 +27,51 @@ use solana_lints::paths;
2727
const ANCHOR_ACCOUNT_GENERIC_ARG_COUNT: usize = 2;
2828

2929
dylint_linting::impl_late_lint! {
30-
/// **What it does:**
30+
/// **What it does:** Checks to make sure there is a key check on identical Anchor accounts.
31+
/// The key check serves to make sure that two identical accounts do not have the same key,
32+
/// ie, they are unique. An Anchor account (`Account<'info, T>`) is identical to another if
33+
/// the generic parameter `T` is the same type for each account.
3134
///
32-
/// **Why is this bad?**
35+
/// **Why is this bad?** If a program contains two identical, mutable Anchor accounts, and
36+
/// performs some operation on those accounts, then a user could pass in the same account
37+
/// twice. Then any previous operations may be overwritten by the last operation, which may
38+
/// not be what the program wanted if it expected different accounts.
3339
///
34-
/// **Known problems:** None.
40+
/// **Known problems:** If a program is not using the anchor #[account] macro constraints,
41+
/// and is instead using checks in the function bodies, and the program uses boolean operator
42+
/// && or || to link constraints in a single if statement, the lint will flag this as a false
43+
/// positive since the lint only catches statements with `==` or `!=`.
44+
/// Another issue is if a program uses an if statement such as `a.key() == b.key()` and then
45+
/// continues to modify the accounts, then this will not be caught. The reason is because the
46+
/// lint regards expressions with `==` as a secure check, since it assumes the program will
47+
/// then return an error (see the secure example). However, it does not explicitly check that
48+
/// an error is returned.
49+
///
50+
/// In general, this lint will catch all vulnerabilities if the anchor macro constraints are
51+
/// used (see the recommended example). It is not as robust if alternative methods are utilized.
52+
/// Thus it is encouraged to use the anchor `#[account]` macro constraints.
3553
///
36-
/// **Example:**
54+
/// **Example:**
3755
///
3856
/// ```rust
39-
/// // example code where a warning is issued
57+
/// #[derive(Accounts)]
58+
/// pub struct Update<'info> {
59+
/// user_a: Account<'info, User>,
60+
/// user_b: Account<'info, User>,
61+
/// }
4062
/// ```
4163
/// Use instead:
4264
/// ```rust
43-
/// // example code that does not raise a warning
65+
/// #[derive(Accounts)]
66+
/// pub struct Update<'info> {
67+
/// #[account(constraint = user_a.key() != user_b.key())]
68+
/// user_a: Account<'info, User>,
69+
/// user_b: Account<'info, User>,
70+
/// }
4471
/// ```
4572
pub DUPLICATE_MUTABLE_ACCOUNTS,
4673
Warn,
47-
"description goes here",
74+
"does not check if multiple identical Anchor accounts have different keys",
4875
DuplicateMutableAccounts::default()
4976
}
5077

@@ -156,7 +183,7 @@ impl<'tcx> LateLintPass<'tcx> for DuplicateMutableAccounts {
156183
cx,
157184
DUPLICATE_MUTABLE_ACCOUNTS,
158185
*first,
159-
"the following expressions have equivalent Account types, yet do not contain a proper key check.",
186+
&format!("the expressions on line {:?} and {:?} have identical Account types, yet do not contain a proper key check.", first, second),
160187
Some(*second),
161188
"add a key check to make sure the accounts have different keys, e.g., x.key() != y.key()",
162189
);

lints/duplicate-mutable-accounts/ui/insecure-2/src/lib.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,12 +10,15 @@ pub mod duplicate_mutable_accounts_insecure {
1010
ctx: Context<Update>,
1111
a: u64,
1212
b: u64,
13+
c: u64,
1314
) -> anchor_lang::solana_program::entrypoint::ProgramResult {
1415
let user_a = &mut ctx.accounts.user_a;
1516
let user_b = &mut ctx.accounts.user_b;
17+
let user_c = &mut ctx.accounts.user_c;
1618

1719
user_a.data = a;
1820
user_b.data = b;
21+
user_c.data = c;
1922
Ok(())
2023
}
2124
}
Lines changed: 24 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1,39 +1,39 @@
1-
error: user_a and user_b have identical account types but do not have a key check constraint
2-
--> $DIR/lib.rs:25:5
1+
error: the expressions on line $DIR/lib.rs:15:27: 15:46 (#0) and $DIR/lib.rs:16:27: 16:46 (#0) have identical Account types, yet do not contain a proper key check.
2+
--> $DIR/lib.rs:15:27
33
|
4-
LL | user_a: Account<'info, User>,
5-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
4+
LL | let user_a = &mut ctx.accounts.user_a;
5+
| ^^^^^^^^^^^^^^^^^^^
66
|
77
= note: `-D duplicate-mutable-accounts` implied by `-D warnings`
8-
help: add an anchor key check constraint: #[account(constraint = user_a.key() != user_b.key())]
9-
--> $DIR/lib.rs:26:5
8+
help: add a key check to make sure the accounts have different keys, e.g., x.key() != y.key()
9+
--> $DIR/lib.rs:16:27
1010
|
11-
LL | user_b: Account<'info, User>,
12-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
11+
LL | let user_b = &mut ctx.accounts.user_b;
12+
| ^^^^^^^^^^^^^^^^^^^
1313

14-
error: user_a and user_c have identical account types but do not have a key check constraint
15-
--> $DIR/lib.rs:25:5
14+
error: the expressions on line $DIR/lib.rs:15:27: 15:46 (#0) and $DIR/lib.rs:17:27: 17:46 (#0) have identical Account types, yet do not contain a proper key check.
15+
--> $DIR/lib.rs:15:27
1616
|
17-
LL | user_a: Account<'info, User>,
18-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
17+
LL | let user_a = &mut ctx.accounts.user_a;
18+
| ^^^^^^^^^^^^^^^^^^^
1919
|
20-
help: add an anchor key check constraint: #[account(constraint = user_a.key() != user_c.key())]
21-
--> $DIR/lib.rs:27:5
20+
help: add a key check to make sure the accounts have different keys, e.g., x.key() != y.key()
21+
--> $DIR/lib.rs:17:27
2222
|
23-
LL | user_c: Account<'info, User>,
24-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
23+
LL | let user_c = &mut ctx.accounts.user_c;
24+
| ^^^^^^^^^^^^^^^^^^^
2525

26-
error: user_b and user_c have identical account types but do not have a key check constraint
27-
--> $DIR/lib.rs:26:5
26+
error: the expressions on line $DIR/lib.rs:16:27: 16:46 (#0) and $DIR/lib.rs:17:27: 17:46 (#0) have identical Account types, yet do not contain a proper key check.
27+
--> $DIR/lib.rs:16:27
2828
|
29-
LL | user_b: Account<'info, User>,
30-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
29+
LL | let user_b = &mut ctx.accounts.user_b;
30+
| ^^^^^^^^^^^^^^^^^^^
3131
|
32-
help: add an anchor key check constraint: #[account(constraint = user_b.key() != user_c.key())]
33-
--> $DIR/lib.rs:27:5
32+
help: add a key check to make sure the accounts have different keys, e.g., x.key() != y.key()
33+
--> $DIR/lib.rs:17:27
3434
|
35-
LL | user_c: Account<'info, User>,
36-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
35+
LL | let user_c = &mut ctx.accounts.user_c;
36+
| ^^^^^^^^^^^^^^^^^^^
3737

3838
error: aborting due to 3 previous errors
3939

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,15 @@
1-
error: user_a and user_b have identical account types but do not have a key check constraint
2-
--> $DIR/lib.rs:25:5
1+
error: the expressions on line $DIR/lib.rs:14:27: 14:46 (#0) and $DIR/lib.rs:15:27: 15:46 (#0) have identical Account types, yet do not contain a proper key check.
2+
--> $DIR/lib.rs:14:27
33
|
4-
LL | user_a: Account<'info, User>,
5-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
4+
LL | let user_a = &mut ctx.accounts.user_a;
5+
| ^^^^^^^^^^^^^^^^^^^
66
|
77
= note: `-D duplicate-mutable-accounts` implied by `-D warnings`
8-
help: add an anchor key check constraint: #[account(constraint = user_a.key() != user_b.key())]
9-
--> $DIR/lib.rs:26:5
8+
help: add a key check to make sure the accounts have different keys, e.g., x.key() != y.key()
9+
--> $DIR/lib.rs:15:27
1010
|
11-
LL | user_b: Account<'info, User>,
12-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
11+
LL | let user_b = &mut ctx.accounts.user_b;
12+
| ^^^^^^^^^^^^^^^^^^^
1313

1414
error: aborting due to previous error
1515

0 commit comments

Comments
 (0)