From 7210d7ea7488251f060294092079cc00f49504fd Mon Sep 17 00:00:00 2001 From: Vara Prasad Bandaru Date: Tue, 5 Mar 2024 22:56:36 +0530 Subject: [PATCH] Add non-anchor tests and update anchor tests --- crate/diffs/missing_signer_check.diff | 6 +++- lints/missing_signer_check/Cargo.lock | 17 +++++----- lints/missing_signer_check/Cargo.toml | 9 ++++++ lints/missing_signer_check/src/lib.rs | 14 ++++++-- .../ui/insecure-non-anchor/Cargo.toml | 21 ++++++++++++ .../ui/insecure-non-anchor/src/lib.rs | 29 +++++++++++++++++ .../ui/insecure-non-anchor/src/lib.stderr | 15 +++++++++ .../ui/insecure/src/lib.stderr | 13 ++++---- .../ui/secure-non-anchor/Cargo.toml | 21 ++++++++++++ .../ui/secure-non-anchor/src/lib.rs | 32 +++++++++++++++++++ .../ui/secure-non-anchor/src/lib.stderr | 0 .../missing_signer_check/ui/secure/src/lib.rs | 2 ++ .../ui/secure/src/lib.stderr | 13 ++++++++ 13 files changed, 174 insertions(+), 18 deletions(-) create mode 100644 lints/missing_signer_check/ui/insecure-non-anchor/Cargo.toml create mode 100644 lints/missing_signer_check/ui/insecure-non-anchor/src/lib.rs create mode 100644 lints/missing_signer_check/ui/insecure-non-anchor/src/lib.stderr create mode 100644 lints/missing_signer_check/ui/secure-non-anchor/Cargo.toml create mode 100644 lints/missing_signer_check/ui/secure-non-anchor/src/lib.rs create mode 100644 lints/missing_signer_check/ui/secure-non-anchor/src/lib.stderr diff --git a/crate/diffs/missing_signer_check.diff b/crate/diffs/missing_signer_check.diff index d6b5f1e..beb3e7f 100644 --- a/crate/diffs/missing_signer_check.diff +++ b/crate/diffs/missing_signer_check.diff @@ -15,6 +15,7 @@ diff -r -x Cargo.lock ./insecure/src/lib.rs ../../../../lints/missing_signer_che > #[allow(dead_code)] > fn main() {} Only in ../../../../lints/missing_signer_check/ui/insecure/src: lib.stderr +Only in ../../../../lints/missing_signer_check/ui: insecure-non-anchor diff -r -x Cargo.lock ./recommended/Cargo.toml ../../../../lints/missing_signer_check/ui/recommended/Cargo.toml 19c19,21 < anchor-lang = "0.20.0" @@ -40,8 +41,11 @@ diff -r -x Cargo.lock ./secure/Cargo.toml ../../../../lints/missing_signer_check diff -r -x Cargo.lock ./secure/src/lib.rs ../../../../lints/missing_signer_check/ui/secure/src/lib.rs 1a2 > use anchor_lang::solana_program::entrypoint::ProgramResult; -21a23,25 +21a23,27 > +> // This is a false positive as the lint does not check for `is_signer` checks if the +> // program is an anchor program. The lint should be updated to remove the false positive. > #[allow(dead_code)] > fn main() {} Only in ../../../../lints/missing_signer_check/ui/secure/src: lib.stderr +Only in ../../../../lints/missing_signer_check/ui: secure-non-anchor diff --git a/lints/missing_signer_check/Cargo.lock b/lints/missing_signer_check/Cargo.lock index f3c1575..3d88fd2 100644 --- a/lints/missing_signer_check/Cargo.lock +++ b/lints/missing_signer_check/Cargo.lock @@ -1456,6 +1456,7 @@ dependencies = [ "dylint_testing", "if_chain", "solana-lints", + "solana-program", ] [[package]] @@ -2036,9 +2037,9 @@ checksum = "e6ecd384b10a64542d77071bd64bd7b231f4ed5940fba55e98c3de13824cf3d7" [[package]] name = "solana-frozen-abi" -version = "1.18.1" +version = "1.18.4" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b24a0e5179387f145afba79d72b27db817cecf1b9494f7cd55d42aa986ed3141" +checksum = "0e608aeeb42922e803589258af0028e2c9e70fbfb04e1f1ff6c5f07019c2af95" dependencies = [ "block-buffer 0.10.4", "bs58 0.4.0", @@ -2061,9 +2062,9 @@ dependencies = [ [[package]] name = "solana-frozen-abi-macro" -version = "1.18.1" +version = "1.18.4" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "92970a9898903eb1433d42f53ca4e8f497bc05382b7bc170ea81d4d3c6ff5d58" +checksum = "1cc7a55ad8a177287f3abb1be371b2a252d4c474d71c2e3983a7dff7db15d3f2" dependencies = [ "proc-macro2", "quote", @@ -2083,9 +2084,9 @@ dependencies = [ [[package]] name = "solana-program" -version = "1.18.1" +version = "1.18.4" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f0ce0a1f4487b0b9e5e53610e8c17e558d17069145281aaae9f825bfef84b16e" +checksum = "91fad730d66a6f33ef5bb180def74a3d84e7487b829a8f67ff58570332481f6c" dependencies = [ "ark-bn254", "ark-ec", @@ -2138,9 +2139,9 @@ dependencies = [ [[package]] name = "solana-sdk-macro" -version = "1.18.1" +version = "1.18.4" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a3e3cdf8616a66e99343c3f99c39f311b4dc3e13977a4c96d7bbaa82dffd2fc5" +checksum = "df952c230e35ba179882cef765655b171b8f99f512f04fc4a84800fa43cfe592" dependencies = [ "bs58 0.4.0", "proc-macro2", diff --git a/lints/missing_signer_check/Cargo.toml b/lints/missing_signer_check/Cargo.toml index 3c2f375..77c59b2 100644 --- a/lints/missing_signer_check/Cargo.toml +++ b/lints/missing_signer_check/Cargo.toml @@ -21,6 +21,14 @@ path = "ui/recommended/src/lib.rs" name = "secure" path = "ui/secure/src/lib.rs" +[[example]] +name = "insecure-non-anchor" +path = "ui/insecure-non-anchor/src/lib.rs" + +[[example]] +name = "secure-non-anchor" +path = "ui/secure-non-anchor/src/lib.rs" + [dependencies] anchor-syn = "0.29.0" clippy_utils = { git = "https://github.com/rust-lang/rust-clippy", rev = "ac4c2094a6030530661bee3876e0228ddfeb6b8b" } @@ -31,6 +39,7 @@ solana-lints = { path = "../../crate" } [dev-dependencies] anchor-lang = "0.29" dylint_testing = "2.6" +solana-program = "1.18.4" [workspace] diff --git a/lints/missing_signer_check/src/lib.rs b/lints/missing_signer_check/src/lib.rs index 636a697..3c7d686 100644 --- a/lints/missing_signer_check/src/lib.rs +++ b/lints/missing_signer_check/src/lib.rs @@ -117,7 +117,7 @@ impl<'tcx> LateLintPass<'tcx> for MissingSignerCheck { /// Return true if any of the expression in body has type `AccountInfo` (`solana_program::account_info::AccountInfo`) fn body_uses_account_info<'tcx>(cx: &LateContext<'tcx>, body: &'tcx Body<'tcx>) -> bool { visit_expr_no_bodies(body.value, |expr| { - let ty = cx.typeck_results().expr_ty(expr); + let ty = cx.typeck_results().expr_ty(expr).peel_refs(); match_type(cx, ty, &paths::SOLANA_PROGRAM_ACCOUNT_INFO) }) } @@ -181,7 +181,7 @@ fn is_is_signer_use<'tcx>(cx: &LateContext<'tcx>, expr: &Expr<'tcx>) -> bool { if let ExprKind::Field(object, field_name) = expr.kind; if field_name.as_str() == "is_signer"; // type of `x` is AccountInfo - let ty = cx.typeck_results().expr_ty(object); + let ty = cx.typeck_results().expr_ty(object).peel_refs(); if match_type(cx, ty, &paths::SOLANA_PROGRAM_ACCOUNT_INFO); then { true @@ -307,3 +307,13 @@ fn recommended() { fn secure() { dylint_testing::ui_test_example(env!("CARGO_PKG_NAME"), "secure"); } + +#[test] +fn insecure_non_anchor() { + dylint_testing::ui_test_example(env!("CARGO_PKG_NAME"), "insecure-non-anchor"); +} + +#[test] +fn secure_non_anchor() { + dylint_testing::ui_test_example(env!("CARGO_PKG_NAME"), "secure-non-anchor"); +} diff --git a/lints/missing_signer_check/ui/insecure-non-anchor/Cargo.toml b/lints/missing_signer_check/ui/insecure-non-anchor/Cargo.toml new file mode 100644 index 0000000..b9324c3 --- /dev/null +++ b/lints/missing_signer_check/ui/insecure-non-anchor/Cargo.toml @@ -0,0 +1,21 @@ +[package] +name = "signer-authorization-insecure-non-anchor" +version = "0.1.0" +description = "" +edition = "2018" + +[lib] +crate-type = ["cdylib", "lib"] +name = "signer_authorization_insecure_non_anchor" + +[features] +no-entrypoint = [] +no-idl = [] +no-log-ix-name = [] +cpi = ["no-entrypoint"] +default = [] + +[dependencies] +solana-program = "1.18.4" + +[workspace] diff --git a/lints/missing_signer_check/ui/insecure-non-anchor/src/lib.rs b/lints/missing_signer_check/ui/insecure-non-anchor/src/lib.rs new file mode 100644 index 0000000..578ed14 --- /dev/null +++ b/lints/missing_signer_check/ui/insecure-non-anchor/src/lib.rs @@ -0,0 +1,29 @@ +use solana_program::msg; +use solana_program::{ + account_info::{next_account_info, AccountInfo}, + entrypoint, + entrypoint::ProgramResult, + program_error::ProgramError, + pubkey::Pubkey, +}; + +entrypoint!(process_instruction); +fn process_instruction( + _program_id: &Pubkey, + accounts: &[AccountInfo], + instruction_data: &[u8], +) -> ProgramResult { + if instruction_data.len() != 0 { + return Err(ProgramError::InvalidInstructionData); + } + log_message(accounts) +} + +pub fn log_message(accounts: &[AccountInfo]) -> ProgramResult { + let authority = next_account_info(&mut accounts.iter())?; + msg!("GM {:?}", authority); + Ok(()) +} + +#[allow(dead_code)] +fn main() {} diff --git a/lints/missing_signer_check/ui/insecure-non-anchor/src/lib.stderr b/lints/missing_signer_check/ui/insecure-non-anchor/src/lib.stderr new file mode 100644 index 0000000..0581054 --- /dev/null +++ b/lints/missing_signer_check/ui/insecure-non-anchor/src/lib.stderr @@ -0,0 +1,15 @@ +error: this function lacks a use of `is_signer` + --> $DIR/lib.rs:22:1 + | +LL | / pub fn log_message(accounts: &[AccountInfo]) -> ProgramResult { +LL | | let authority = next_account_info(&mut accounts.iter())?; +LL | | msg!("GM {:?}", authority); +LL | | Ok(()) +LL | | } + | |_^ + | + = note: `-D missing-signer-check` implied by `-D warnings` + = help: to override `-D warnings` add `#[allow(missing_signer_check)]` + +error: aborting due to 1 previous error + diff --git a/lints/missing_signer_check/ui/insecure/src/lib.stderr b/lints/missing_signer_check/ui/insecure/src/lib.stderr index ef7baf6..b55affc 100644 --- a/lints/missing_signer_check/ui/insecure/src/lib.stderr +++ b/lints/missing_signer_check/ui/insecure/src/lib.stderr @@ -1,11 +1,10 @@ -error: this function lacks a use of `is_signer` - --> $DIR/lib.rs:10:5 +error: Account `authority` might need to be a signer + --> $DIR/lib.rs:18:5 | -LL | / pub fn log_message(ctx: Context) -> ProgramResult { -LL | | msg!("GM {}", ctx.accounts.authority.key().to_string()); -LL | | Ok(()) -LL | | } - | |_____^ +LL | pub struct LogMessage<'info> { + | ---------- Accounts of this instruction +LL | authority: AccountInfo<'info>, + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | = note: `-D missing-signer-check` implied by `-D warnings` = help: to override `-D warnings` add `#[allow(missing_signer_check)]` diff --git a/lints/missing_signer_check/ui/secure-non-anchor/Cargo.toml b/lints/missing_signer_check/ui/secure-non-anchor/Cargo.toml new file mode 100644 index 0000000..7a777ca --- /dev/null +++ b/lints/missing_signer_check/ui/secure-non-anchor/Cargo.toml @@ -0,0 +1,21 @@ +[package] +name = "signer-authorization-secure-non-anchor" +version = "0.1.0" +description = "" +edition = "2018" + +[lib] +crate-type = ["cdylib", "lib"] +name = "signer_authorization_secure_non_anchor" + +[features] +no-entrypoint = [] +no-idl = [] +no-log-ix-name = [] +cpi = ["no-entrypoint"] +default = [] + +[dependencies] +solana-program = "1.18.4" + +[workspace] diff --git a/lints/missing_signer_check/ui/secure-non-anchor/src/lib.rs b/lints/missing_signer_check/ui/secure-non-anchor/src/lib.rs new file mode 100644 index 0000000..6894cc1 --- /dev/null +++ b/lints/missing_signer_check/ui/secure-non-anchor/src/lib.rs @@ -0,0 +1,32 @@ +use solana_program::msg; +use solana_program::{ + account_info::{next_account_info, AccountInfo}, + entrypoint, + entrypoint::ProgramResult, + program_error::ProgramError, + pubkey::Pubkey, +}; + +entrypoint!(process_instruction); +fn process_instruction( + _program_id: &Pubkey, + accounts: &[AccountInfo], + instruction_data: &[u8], +) -> ProgramResult { + if instruction_data.len() != 0 { + return Err(ProgramError::InvalidInstructionData); + } + log_message(accounts) +} + +pub fn log_message(accounts: &[AccountInfo]) -> ProgramResult { + let authority = next_account_info(&mut accounts.iter())?; + if !authority.is_signer { + return Err(ProgramError::MissingRequiredSignature); + } + msg!("GM {:?}", authority); + Ok(()) +} + +#[allow(dead_code)] +fn main() {} diff --git a/lints/missing_signer_check/ui/secure-non-anchor/src/lib.stderr b/lints/missing_signer_check/ui/secure-non-anchor/src/lib.stderr new file mode 100644 index 0000000..e69de29 diff --git a/lints/missing_signer_check/ui/secure/src/lib.rs b/lints/missing_signer_check/ui/secure/src/lib.rs index 8181675..f2fb800 100644 --- a/lints/missing_signer_check/ui/secure/src/lib.rs +++ b/lints/missing_signer_check/ui/secure/src/lib.rs @@ -21,5 +21,7 @@ pub struct LogMessage<'info> { authority: AccountInfo<'info>, } +// This is a false positive as the lint does not check for `is_signer` checks if the +// program is an anchor program. The lint should be updated to remove the false positive. #[allow(dead_code)] fn main() {} diff --git a/lints/missing_signer_check/ui/secure/src/lib.stderr b/lints/missing_signer_check/ui/secure/src/lib.stderr index e69de29..a7ad211 100644 --- a/lints/missing_signer_check/ui/secure/src/lib.stderr +++ b/lints/missing_signer_check/ui/secure/src/lib.stderr @@ -0,0 +1,13 @@ +error: Account `authority` might need to be a signer + --> $DIR/lib.rs:21:5 + | +LL | pub struct LogMessage<'info> { + | ---------- Accounts of this instruction +LL | authority: AccountInfo<'info>, + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: `-D missing-signer-check` implied by `-D warnings` + = help: to override `-D warnings` add `#[allow(missing_signer_check)]` + +error: aborting due to 1 previous error +