diff --git a/README.md b/README.md index 66d31ad2..11152d57 100644 --- a/README.md +++ b/README.md @@ -8,15 +8,15 @@ The lints are inspired by the [Sealevel Attacks]. (See also @pencilflip's [Twitt The current lints are: -| Library | Description | -| ---------------------------------------------------------------- | ---------------------------------------------------------------------------------------------------------------------------------------- | -| [`arbitrary_cpi`](lints/arbitrary_cpi) | lint for [5-arbitrary-cpi](https://github.com/coral-xyz/sealevel-attacks/tree/master/programs/5-arbitrary-cpi) | -| [`bump_seed_canonicalization`](lints/bump_seed_canonicalization) | lint for [6-bump-seed-canonicalization](https://github.com/coral-xyz/sealevel-attacks/tree/master/programs/7-bump-seed-canonicalization) | -| [`insecure_account_close`](lints/insecure_account_close) | lint for [9-closing-accounts](https://github.com/coral-xyz/sealevel-attacks/tree/master/programs/9-closing-accounts) | -| [`missing_owner_check`](lints/missing_owner_check) | lint for [2-owner-checks](https://github.com/coral-xyz/sealevel-attacks/tree/master/programs/2-owner-checks) | -| [`missing_signer_check`](lints/missing_signer_check) | lint for [0-signer-authorization](https://github.com/coral-xyz/sealevel-attacks/tree/master/programs/0-signer-authorization) | -| [`sysvar_get`](lints/sysvar_get) | Reports uses of `Sysvar::from_account_info` instead of `Sysvar::get` | -| [`type_cosplay`](lints/type_cosplay) | lint for [3-type-cosplay](https://github.com/coral-xyz/sealevel-attacks/tree/master/programs/3-type-cosplay) | +| Library | Description | Anchor | Non Anchor | +| ---------------------------------------------------------------- | ---------------------------------------------------------------------------------------------------------------------------------------- | ------------------ | ------------------ | +| [`arbitrary_cpi`](lints/arbitrary_cpi) | lint for [5-arbitrary-cpi](https://github.com/coral-xyz/sealevel-attacks/tree/master/programs/5-arbitrary-cpi) | :heavy_check_mark: | :heavy_check_mark: | +| [`bump_seed_canonicalization`](lints/bump_seed_canonicalization) | lint for [6-bump-seed-canonicalization](https://github.com/coral-xyz/sealevel-attacks/tree/master/programs/7-bump-seed-canonicalization) | | :heavy_check_mark: | +| [`insecure_account_close`](lints/insecure_account_close) | lint for [9-closing-accounts](https://github.com/coral-xyz/sealevel-attacks/tree/master/programs/9-closing-accounts) | :heavy_check_mark: | :heavy_check_mark: | +| [`missing_owner_check`](lints/missing_owner_check) | lint for [2-owner-checks](https://github.com/coral-xyz/sealevel-attacks/tree/master/programs/2-owner-checks) | :heavy_check_mark: | :heavy_check_mark: | +| [`missing_signer_check`](lints/missing_signer_check) | lint for [0-signer-authorization](https://github.com/coral-xyz/sealevel-attacks/tree/master/programs/0-signer-authorization) | :heavy_check_mark: | :heavy_check_mark: | +| [`sysvar_get`](lints/sysvar_get) | Reports uses of `Sysvar::from_account_info` instead of `Sysvar::get` | :heavy_check_mark: | :heavy_check_mark: | +| [`type_cosplay`](lints/type_cosplay) | lint for [3-type-cosplay](https://github.com/coral-xyz/sealevel-attacks/tree/master/programs/3-type-cosplay) | | :heavy_check_mark: | ## Usage diff --git a/lints/arbitrary_cpi/README.md b/lints/arbitrary_cpi/README.md index e923e43e..99a42b77 100644 --- a/lints/arbitrary_cpi/README.md +++ b/lints/arbitrary_cpi/README.md @@ -6,6 +6,11 @@ Finds uses of solana_program::program::invoke that do not check the program_id **Why is this bad?** A contract could call into an attacker-controlled contract instead of the intended one +**Works on:** + +- [x] Anchor +- [x] Non Anchor + **Known problems:** False positives, since the program_id check may be within some other function (does not trace through function calls) diff --git a/lints/arbitrary_cpi/src/lib.rs b/lints/arbitrary_cpi/src/lib.rs index 08639e23..cafc4892 100644 --- a/lints/arbitrary_cpi/src/lib.rs +++ b/lints/arbitrary_cpi/src/lib.rs @@ -26,6 +26,11 @@ dylint_linting::declare_late_lint! { /// **Why is this bad?** /// A contract could call into an attacker-controlled contract instead of the intended one /// + /// **Works on:** + /// + /// - [x] Anchor + /// - [x] Non Anchor + /// /// **Known problems:** /// False positives, since the program_id check may be within some other function (does not /// trace through function calls) diff --git a/lints/bump_seed_canonicalization/README.md b/lints/bump_seed_canonicalization/README.md index d7ba6c07..1a1dd73b 100644 --- a/lints/bump_seed_canonicalization/README.md +++ b/lints/bump_seed_canonicalization/README.md @@ -11,6 +11,11 @@ able to pick the bump_seed, since that would result in a different address. See https://github.com/crytic/building-secure-contracts/tree/master/not-so-smart-contracts/solana/improper_pda_validation +**Works on:** + +- [ ] Anchor +- [x] Non Anchor + **Known problems:** False positives, since the bump_seed check may be within some other function (does not diff --git a/lints/bump_seed_canonicalization/src/lib.rs b/lints/bump_seed_canonicalization/src/lib.rs index 88a751e7..96ae9936 100644 --- a/lints/bump_seed_canonicalization/src/lib.rs +++ b/lints/bump_seed_canonicalization/src/lib.rs @@ -36,6 +36,11 @@ dylint_linting::declare_late_lint! { /// /// See https://github.com/crytic/building-secure-contracts/tree/master/not-so-smart-contracts/solana/improper_pda_validation /// + /// **Works on:** + /// + /// - [ ] Anchor + /// - [x] Non Anchor + /// /// **Known problems:** /// /// False positives, since the bump_seed check may be within some other function (does not diff --git a/lints/insecure_account_close/README.md b/lints/insecure_account_close/README.md index 8cfb4d77..c93c20a1 100644 --- a/lints/insecure_account_close/README.md +++ b/lints/insecure_account_close/README.md @@ -11,6 +11,11 @@ See: https://docs.solana.com/developing/programming-model/transactions#multiple- > An example of where this could be a problem is if a token program, upon transferring the token out of an account, sets the account's lamports to zero, assuming it will be deleted by the runtime. If the program does not zero out the account's data, a malicious user could trail this instruction with another that transfers the tokens a second time. +**Works on:** + +- [x] Anchor +- [x] Non Anchor + **Known problems:** None diff --git a/lints/insecure_account_close/src/lib.rs b/lints/insecure_account_close/src/lib.rs index 5d969d77..c9aee816 100644 --- a/lints/insecure_account_close/src/lib.rs +++ b/lints/insecure_account_close/src/lib.rs @@ -26,6 +26,11 @@ dylint_linting::declare_late_lint! { /// /// > An example of where this could be a problem is if a token program, upon transferring the token out of an account, sets the account's lamports to zero, assuming it will be deleted by the runtime. If the program does not zero out the account's data, a malicious user could trail this instruction with another that transfers the tokens a second time. /// + /// **Works on:** + /// + /// - [x] Anchor + /// - [x] Non Anchor + /// /// **Known problems:** /// /// None diff --git a/lints/missing_owner_check/README.md b/lints/missing_owner_check/README.md index 6f026e6c..11ab35c7 100644 --- a/lints/missing_owner_check/README.md +++ b/lints/missing_owner_check/README.md @@ -19,6 +19,11 @@ If no owner check is done on the account, then a malicious actor could pass in a account owned by some other program. The code may then perform some actions on the unauthorized account that is not owned by the SPL Token program. +**Works on:** + +- [x] Anchor +- [x] Non Anchor + **Known problems:** Key checks can be strengthened. Currently, the lint only checks that the account's owner diff --git a/lints/missing_owner_check/src/lib.rs b/lints/missing_owner_check/src/lib.rs index 33819e2a..6482ef36 100644 --- a/lints/missing_owner_check/src/lib.rs +++ b/lints/missing_owner_check/src/lib.rs @@ -41,6 +41,11 @@ dylint_linting::impl_late_lint! { /// account owned by some other program. The code may then perform some actions on the /// unauthorized account that is not owned by the SPL Token program. /// + /// **Works on:** + /// + /// - [x] Anchor + /// - [x] Non Anchor + /// /// **Known problems:** /// /// Key checks can be strengthened. Currently, the lint only checks that the account's owner diff --git a/lints/missing_signer_check/README.md b/lints/missing_signer_check/README.md index ac8b7f57..6d729c87 100644 --- a/lints/missing_signer_check/README.md +++ b/lints/missing_signer_check/README.md @@ -14,6 +14,11 @@ then anyone can create the instruction, call the program and perform a privilege For example if the Token program does not check that the owner of the tokens is a signer in the transfer instruction then anyone can transfer the tokens and steal them. +**Works on:** + +- [x] Anchor +- [x] Non Anchor + **Known problems:** None. diff --git a/lints/missing_signer_check/src/lib.rs b/lints/missing_signer_check/src/lib.rs index 3c7d6866..3b401458 100644 --- a/lints/missing_signer_check/src/lib.rs +++ b/lints/missing_signer_check/src/lib.rs @@ -34,6 +34,11 @@ dylint_linting::impl_late_lint! { /// For example if the Token program does not check that the owner of the tokens is a signer in the transfer instruction then anyone can /// transfer the tokens and steal them. /// + /// **Works on:** + /// + /// - [x] Anchor + /// - [x] Non Anchor + /// /// **Known problems:** /// None. /// diff --git a/lints/sysvar_get/README.md b/lints/sysvar_get/README.md index 1ea20b74..fd597686 100644 --- a/lints/sysvar_get/README.md +++ b/lints/sysvar_get/README.md @@ -28,6 +28,11 @@ References: [`solana_program/sysvar` docs](https://docs.rs/solana-program/latest/solana_program/sysvar/index.html#:~:text=programs%20should%20prefer%20to%20call%20Sysvar%3A%3Aget), [Anchor docs](https://docs.rs/anchor-lang/latest/anchor_lang/accounts/sysvar/struct.Sysvar.html#:~:text=If%20possible%2C%20sysvars%20should%20not%20be%20used%20via%20accounts) +**Works on:** + +- [x] Anchor +- [x] Non Anchor + **Known problems:** None diff --git a/lints/sysvar_get/src/lib.rs b/lints/sysvar_get/src/lib.rs index 9c17cc08..7555d8ad 100644 --- a/lints/sysvar_get/src/lib.rs +++ b/lints/sysvar_get/src/lib.rs @@ -48,6 +48,11 @@ dylint_linting::declare_late_lint! { /// [`solana_program/sysvar` docs](https://docs.rs/solana-program/latest/solana_program/sysvar/index.html#:~:text=programs%20should%20prefer%20to%20call%20Sysvar%3A%3Aget), /// [Anchor docs](https://docs.rs/anchor-lang/latest/anchor_lang/accounts/sysvar/struct.Sysvar.html#:~:text=If%20possible%2C%20sysvars%20should%20not%20be%20used%20via%20accounts) /// + /// **Works on:** + /// + /// - [x] Anchor + /// - [x] Non Anchor + /// /// **Known problems:** /// /// None diff --git a/lints/type_cosplay/README.md b/lints/type_cosplay/README.md index d04983f0..3083bf32 100644 --- a/lints/type_cosplay/README.md +++ b/lints/type_cosplay/README.md @@ -27,6 +27,11 @@ tell the difference between deserialized type `X` and deserialized type `Y`. Thi malicious user to substitute `X` for `Y` or vice versa, and the code may perform unauthorized actions with the bytes. +**Works on:** + +- [ ] Anchor +- [x] Non Anchor + **Known problems:** In the case when only one enum is deserialized, this lint by default diff --git a/lints/type_cosplay/src/lib.rs b/lints/type_cosplay/src/lib.rs index df094431..30727698 100644 --- a/lints/type_cosplay/src/lib.rs +++ b/lints/type_cosplay/src/lib.rs @@ -53,6 +53,11 @@ dylint_linting::impl_late_lint! { /// malicious user to substitute `X` for `Y` or vice versa, and the code may perform unauthorized /// actions with the bytes. /// + /// **Works on:** + /// + /// - [ ] Anchor + /// - [x] Non Anchor + /// /// **Known problems:** /// /// In the case when only one enum is deserialized, this lint by default diff --git a/scripts/update_readmes.sh b/scripts/update_readmes.sh index 0be168a5..ad6bd546 100755 --- a/scripts/update_readmes.sh +++ b/scripts/update_readmes.sh @@ -16,6 +16,22 @@ WORKSPACE="$(realpath "$SCRIPTS"/..)" cd "$WORKSPACE"/lints +for LIBRARY in *; do + pushd "$LIBRARY" >/dev/null + + ( + echo "# $LIBRARY" + echo + cat src/*.rs | + sed -n '/^[a-z_:]*_lint! {$/,/^}$/p' | + sed -n 's,^[[:space:]]*///\([[:space:]]\(.*\)\)\?$,\2,;T;p' + ) > README.md + + # prettier --write README.md + + popd >/dev/null +done + TMP="$(mktemp)" LISTED= @@ -25,10 +41,31 @@ cat ../README.md | while read X; do if [[ "$X" =~ ^\| ]]; then if [[ -z "$LISTED" ]]; then - echo '| Library | Description |' - echo '| - | - |' - grep -H '^description = "[^"]*"$' */Cargo.toml | - sed 's,^\([^/]*\)/Cargo.toml:description = "\([^"]*\)"$,| [`\1`](lints/\1) | \2 |,' + echo '| Library | Description | Anchor | Non Anchor |' + echo '| - | - | - | - |' + for DIR in */; do + CARGO_TOML="${DIR}Cargo.toml" + README="${DIR}README.md" + DESC=$( + grep -H '^description = "[^"]*"$' "$CARGO_TOML" | + sed 's,^\([^/]*\)/Cargo.toml:description = "\([^"]*\)"$,| [`\1`](lints/\1) | \2,' + ) + ANCHOR=$( + grep '^- \[[ x]\] Anchor$' "$README" | cut -d "[" -f2 | cut -d "]" -f1 + ) + NON_ANCHOR=$( + grep '^- \[[ x]\] Non Anchor$' "$README" | cut -d "[" -f2 | cut -d "]" -f1 + ) + ANCHOR_COLUMN=" " + if [[ "$ANCHOR" == "x" ]]; then + ANCHOR_COLUMN=":heavy_check_mark:" + fi + NON_ANCHOR_COLUMN=" " + if [[ "$NON_ANCHOR" == "x" ]]; then + NON_ANCHOR_COLUMN=":heavy_check_mark:" + fi + echo "$DESC | $ANCHOR_COLUMN | $NON_ANCHOR_COLUMN |" + done LISTED=1 fi continue @@ -40,19 +77,3 @@ cat > "$TMP" mv "$TMP" ../README.md prettier --write ../README.md - -for LIBRARY in *; do - pushd "$LIBRARY" >/dev/null - - ( - echo "# $LIBRARY" - echo - cat src/*.rs | - sed -n '/^[a-z_:]*_lint! {$/,/^}$/p' | - sed -n 's,^[[:space:]]*///\([[:space:]]\(.*\)\)\?$,\2,;T;p' - ) > README.md - - # prettier --write README.md - - popd >/dev/null -done