Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix missing_owner_check lint #71

Merged
merged 1 commit into from
Jan 12, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions crate/src/paths.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,15 @@
// all in upper snake case.

pub const ANCHOR_LANG_ACCOUNT: [&str; 4] = ["anchor_lang", "accounts", "account", "Account"];
pub const ANCHOR_LANG_ACCOUNT_LOADER: [&str; 4] = ["anchor_lang", "accounts", "account_loader", "AccountLoader"];
pub const ANCHOR_LANG_PROGRAM: [&str; 4] = ["anchor_lang", "accounts", "program", "Program"];
pub const ANCHOR_LANG_SYSTEM_ACCOUNT: [&str; 4] =
["anchor_lang", "accounts", "system_account", "SystemAccount"];
pub const ANCHOR_LANG_ACCOUNT_DESERIALIZE: [&str; 2] = ["anchor_lang", "AccountDeserialize"];
pub const ANCHOR_LANG_CONTEXT: [&str; 3] = ["anchor_lang", "context", "Context"];
pub const ANCHOR_LANG_DISCRIMINATOR: [&str; 2] = ["anchor_lang", "Discriminator"];
pub const ANCHOR_LANG_SIGNER: [&str; 4] = ["anchor_lang", "accounts", "signer", "Signer"];
pub const ANCHOR_LANG_SYSVAR: [&str; 4] = ["anchor_lang", "accounts", "sysvar", "Sysvar"];
pub const ANCHOR_LANG_TO_ACCOUNT_INFO: [&str; 3] =
["anchor_lang", "ToAccountInfo", "to_account_info"];
pub const ANCHOR_LANG_TRY_DESERIALIZE: [&str; 3] =
Expand All @@ -18,6 +20,7 @@ pub const ANCHOR_LANG_TRY_DESERIALIZE: [&str; 3] =
pub const BORSH_TRY_FROM_SLICE: [&str; 4] = ["borsh", "de", "BorshDeserialize", "try_from_slice"];

pub const CORE_BRANCH: [&str; 5] = ["core", "ops", "try_trait", "Try", "branch"];
pub const CORE_CLONE: [&str; 4] = ["core", "clone", "Clone", "clone"];

pub const SOLANA_PROGRAM_ACCOUNT_INFO: [&str; 3] =
["solana_program", "account_info", "AccountInfo"];
Expand Down
25 changes: 23 additions & 2 deletions lints/missing_owner_check/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,7 @@ fn get_referenced_accounts<'tcx>(
impl<'cx, 'tcx> Visitor<'tcx> for AccountUses<'cx, 'tcx> {
fn visit_expr(&mut self, expr: &'tcx Expr<'tcx>) {
if_chain! {
if !is_call_to_clone(self.cx, expr);
let ty = self.cx.typeck_results().expr_ty(expr);
if match_type(self.cx, ty, &paths::SOLANA_PROGRAM_ACCOUNT_INFO);
if !is_safe_to_account_info(self.cx, expr);
Expand All @@ -117,25 +118,45 @@ impl<'cx, 'tcx> Visitor<'tcx> for AccountUses<'cx, 'tcx> {
}
}

/// Return true if the expr is a method call to clone else false
fn is_call_to_clone<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) -> bool {
if_chain! {
if let ExprKind::MethodCall(_, _, _, _) = expr.kind;
if let Some(def_id) = cx.typeck_results().type_dependent_def_id(expr.hir_id);
if match_any_def_paths(cx, def_id, &[&paths::CORE_CLONE]).is_some();
then {
true
} else {
false
}
}
}

// smoelius: See: https://github.com/crytic/solana-lints/issues/31
fn is_safe_to_account_info<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) -> bool {
if_chain! {
if let Some(recv) = is_to_account_info(cx, expr);
let recv_ty = cx.typeck_results().expr_ty(recv);
if let Some(recv) = is_to_account_info(cx, expr);
if let ty::Ref(_, recv_ty, _) = cx.typeck_results().expr_ty_adjusted(recv).kind();
if let ty::Adt(adt_def, _) = recv_ty.kind();
// smoelius:
// - `Account` requires its type argument to implement `anchor_lang::Owner`.
// - `Program`'s implementation of `try_from` checks the account's program id. So there is
// no ambiguity in regard to the account's owner.
// - `SystemAccount`'s implementation of `try_from` checks that the account's owner is the
// System Program.
// - `AccountLoader` requires its type argument to implement `anchor_lang::Owner`.
// - `Signer` are mostly accounts with a private key and most of the times owned by System Program.
// - `Sysvar` type arguments checks the account key.
if match_any_def_paths(
cx,
adt_def.did(),
&[
&paths::ANCHOR_LANG_ACCOUNT,
&paths::ANCHOR_LANG_PROGRAM,
&paths::ANCHOR_LANG_SYSTEM_ACCOUNT,
&paths::ANCHOR_LANG_ACCOUNT_LOADER,
&paths::ANCHOR_LANG_SIGNER,
&paths::ANCHOR_LANG_SYSVAR,
],
)
.is_some();
Expand Down
Loading