Skip to content
Closed
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
129 changes: 79 additions & 50 deletions lang/syn/src/parser/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,34 +52,92 @@ impl CrateContext {
pub fn safety_checks(&self) -> Result<()> {
// Check all structs for unsafe field types, i.e. AccountInfo and UncheckedAccount.
for ctx in self.modules.values() {
for unsafe_field in ctx.unsafe_struct_fields() {
// Check if unsafe field type has been documented with a /// SAFETY: doc string.
let is_documented = unsafe_field.attrs.iter().any(|attr| {
attr.tokens.clone().into_iter().any(|token| match token {
// Check for doc comments containing CHECK
proc_macro2::TokenTree::Literal(s) => s.to_string().contains("CHECK"),
_ => false,
})
for struct_item in ctx.structs() {
// Only check structs with #[derive(Accounts)]
let has_accounts_derive = struct_item.attrs.iter().any(|attr| {
match attr.parse_meta() {
Ok(syn::Meta::List(syn::MetaList{path, nested, ..})) => {
path.is_ident("derive") && nested.iter().any(|nested| {
matches!(nested, syn::NestedMeta::Meta(syn::Meta::Path(path)) if path.is_ident("Accounts"))
})
}
_ => false
}
});
if !is_documented {
let ident = unsafe_field.ident.as_ref().unwrap();
let span = ident.span();
// Error if undocumented.
return Err(anyhow!(
r#"

if has_accounts_derive {
// Check for duplicate field names within the same struct
let mut field_names = std::collections::HashSet::new();
for field in &struct_item.fields {
if let Some(ident) = &field.ident {
let field_name = ident.to_string();
if !field_names.insert(field_name.clone()) {
let span = ident.span();
return Err(anyhow!(
r#"
{}:{}:{}
Struct field "{}" is unsafe, but is not documented.
Duplicate field name "{}" found in struct "{}".
Each field in an accounts struct must have a unique name.
Please rename one of the duplicate fields to resolve this conflict.
"#,
ctx.file.canonicalize().unwrap().display(),
span.start().line,
span.start().column,
field_name,
struct_item.ident.to_string()
));
}
}
}

for field in &struct_item.fields {
// Check if this field is an unsafe type
let is_unsafe_type = match &field.ty {
syn::Type::Path(syn::TypePath {
path: syn::Path { segments, .. },
..
}) => {
segments.len() == 1 && segments[0].ident == "UncheckedAccount"
|| segments[0].ident == "AccountInfo"
}
_ => false,
};

if is_unsafe_type {
// Check if unsafe field type has been documented with a /// CHECK: doc string.
let is_documented = field.attrs.iter().any(|attr| {
attr.tokens.clone().into_iter().any(|token| match token {
// Check for doc comments containing CHECK
proc_macro2::TokenTree::Literal(s) => {
s.to_string().contains("CHECK")
}
_ => false,
})
});

if !is_documented {
let ident = field.ident.as_ref().unwrap();
let span = ident.span();
// Error if undocumented.
return Err(anyhow!(
r#"
{}:{}:{}
Struct field "{}" in struct "{}" is unsafe, but is not documented.
Please add a `/// CHECK:` doc comment explaining why no checks through types are necessary.
Alternatively, for reasons like quick prototyping, you may disable the safety checks
by using the `skip-lint` option.
See https://www.anchor-lang.com/docs/the-accounts-struct#safety-checks for more information.
"#,
ctx.file.canonicalize().unwrap().display(),
span.start().line,
span.start().column,
ident.to_string()
));
};
ctx.file.canonicalize().unwrap().display(),
span.start().line,
span.start().column,
ident.to_string(),
struct_item.ident.to_string()
));
}
}
}
}
}
}
Ok(())
Expand Down Expand Up @@ -221,35 +279,6 @@ impl ParsedModule {
})
}

fn unsafe_struct_fields(&self) -> impl Iterator<Item = &syn::Field> {
let accounts_filter = |item_struct: &&syn::ItemStruct| {
item_struct.attrs.iter().any(|attr| {
match attr.parse_meta() {
Ok(syn::Meta::List(syn::MetaList{path, nested, ..})) => {
path.is_ident("derive") && nested.iter().any(|nested| {
matches!(nested, syn::NestedMeta::Meta(syn::Meta::Path(path)) if path.is_ident("Accounts"))
})
}
_ => false
}
})
};

self.structs()
.filter(accounts_filter)
.flat_map(|s| &s.fields)
.filter(|f| match &f.ty {
syn::Type::Path(syn::TypePath {
path: syn::Path { segments, .. },
..
}) => {
segments.len() == 1 && segments[0].ident == "UncheckedAccount"
|| segments[0].ident == "AccountInfo"
}
_ => false,
})
}

fn enums(&self) -> impl Iterator<Item = &syn::ItemEnum> {
self.items.iter().filter_map(|i| match i {
syn::Item::Enum(item) => Some(item),
Expand Down
20 changes: 20 additions & 0 deletions tests/safety-checks/programs/duplicate-names/Cargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
[package]
name = "duplicate-names"
version = "0.1.0"
description = "Created with Anchor"
edition = "2021"

[lib]
crate-type = ["cdylib", "lib"]
name = "duplicate_names"

[features]
no-entrypoint = []
no-idl = []
no-log-ix-name = []
cpi = ["no-entrypoint"]
default = []
idl-build = ["anchor-lang/idl-build"]

[dependencies]
anchor-lang = { path = "../../../../lang", features = ["init-if-needed"] }
6 changes: 6 additions & 0 deletions tests/safety-checks/programs/duplicate-names/Xargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
[target.bpf]
linker = "rust-lld"
rustflags = [
"-C", "link-arg=--import-memory",
"-C", "link-arg=--export-table",
]
25 changes: 25 additions & 0 deletions tests/safety-checks/programs/duplicate-names/src/lib.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
use anchor_lang::prelude::*;

declare_id!("Fg6PaFpoGXkYsidMpWTK6W2BeZ7FEfcYkg476zPFsLnS");

#[program]
pub mod duplicate_names {
use super::*;
pub fn func_one(ctx: Context<FuncOne>) -> Result<()> {
Ok(())
}

pub fn func_two(ctx: Context<FuncTwo>) -> Result<()> {
Ok(())
}
}

#[derive(Accounts)]
pub struct FuncOne<'info> {
my_account: UncheckedAccount<'info>,
}

#[derive(Accounts)]
pub struct FuncTwo<'info> {
my_account: UncheckedAccount<'info>,
}
15 changes: 13 additions & 2 deletions tests/safety-checks/test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ echo "Building programs"
#
pushd programs/unchecked-account/
output=$(anchor build 2>&1 > /dev/null)
if ! [[ $output =~ "Struct field \"unchecked\" is unsafe" ]]; then
if ! [[ $output =~ "Struct field \"unchecked\" in struct \"Initialize\" is unsafe" ]]; then
echo "Error: expected /// CHECK error"
exit 1
fi
Expand All @@ -18,12 +18,23 @@ popd
#
pushd programs/account-info/
output=$(anchor build 2>&1 > /dev/null)
if ! [[ $output =~ "Struct field \"unchecked\" is unsafe" ]]; then
if ! [[ $output =~ "Struct field \"unchecked\" in struct \"Initialize\" is unsafe" ]]; then
echo "Error: expected /// CHECK error"
exit 1
fi
popd

#
# Build the duplicate-names variant.
#
pushd programs/duplicate-names/
output=$(anchor build 2>&1 > /dev/null)
if ! [[ $output =~ "Struct field \"my_account\" in struct \"FuncOne\" is unsafe" ]]; then
echo "Error: expected /// CHECK error for duplicate-names"
exit 1
fi
popd

#
# Build the control variant.
#
Expand Down