diff --git a/rust/src/nasl/builtin/description/mod.rs b/rust/src/nasl/builtin/description/mod.rs index 02d95c95ff..58d961a181 100644 --- a/rust/src/nasl/builtin/description/mod.rs +++ b/rust/src/nasl/builtin/description/mod.rs @@ -130,12 +130,32 @@ pub fn script_tag(ctx: &ScanCtx, name: &str, value: &NaslValue) -> Result<(), Fn Ok(()) } -#[nasl_function(named(name, value))] -pub fn script_xref(ctx: &ScanCtx, name: String, value: String) { - ctx.nvt_mut().as_mut().unwrap().references.push(NvtRef { - class: name, - id: value, - }); +#[nasl_function(named(name, value, csv))] +pub fn script_xref( + ctx: &ScanCtx, + name: String, + value: Option, + csv: Option, +) -> Result<(), FnError> { + if value.is_none() && csv.is_none() { + return Err(FnError::missing_argument("value or csv")); + } + + let mut nvt = ctx.nvt_mut(); + let references = &mut nvt.as_mut().unwrap().references; + if let Some(csv) = csv { + references.extend(csv.split(',').map(|id| NvtRef { + class: name.clone(), + id: id.to_owned(), + })); + } + if let Some(value) = value { + references.push(NvtRef { + class: name, + id: value, + }); + } + Ok(()) } #[nasl_function(named(name, value, id, r#type))] diff --git a/rust/src/nasl/builtin/misc/mod.rs b/rust/src/nasl/builtin/misc/mod.rs index a60634e56f..b573f6754b 100644 --- a/rust/src/nasl/builtin/misc/mod.rs +++ b/rust/src/nasl/builtin/misc/mod.rs @@ -343,6 +343,11 @@ impl DefineGlobalVars for Misc { ), ("NASL_ERR_EUNREACH", NaslValue::Number(NASL_ERR_EUNREACH)), ("NASL_ERR_EUNKNOWN", NaslValue::Number(NASL_ERR_EUNKNOWN)), + ("NOERR", NaslValue::Number(NASL_ERR_NOERR)), + ("ETIMEDOUT", NaslValue::Number(NASL_ERR_ETIMEDOUT)), + ("ECONNRESET", NaslValue::Number(NASL_ERR_ECONNRESET)), + ("EUNREACH", NaslValue::Number(NASL_ERR_EUNREACH)), + ("EUNKNOWN", NaslValue::Number(NASL_ERR_EUNKNOWN)), ] } } diff --git a/rust/src/nasl/builtin/network/socket.rs b/rust/src/nasl/builtin/network/socket.rs index 736ed9f766..b791251f7f 100644 --- a/rust/src/nasl/builtin/network/socket.rs +++ b/rust/src/nasl/builtin/network/socket.rs @@ -1412,6 +1412,7 @@ function_set! { impl DefineGlobalVars for SocketFns { fn get_global_vars() -> Vec<(&'static str, NaslValue)> { vec![ + ("MSG_OOB", NaslValue::Number(libc::MSG_OOB.into())), ("ENCAPS_AUTO", NaslValue::Number(OpenvasEncaps::Auto.into())), ("ENCAPS_IP", NaslValue::Number(OpenvasEncaps::Ip.into())), ( diff --git a/rust/src/nasl/builtin/raw_ip/packet_forgery.rs b/rust/src/nasl/builtin/raw_ip/packet_forgery.rs index 4b4b995b32..39fc6033c6 100644 --- a/rust/src/nasl/builtin/raw_ip/packet_forgery.rs +++ b/rust/src/nasl/builtin/raw_ip/packet_forgery.rs @@ -3373,6 +3373,10 @@ impl DefineGlobalVars for PacketForgery { "IPPROTO_ICMP", NaslValue::Number(IpNextHeaderProtocols::Icmp.to_primitive_values().0.into()), ), + ( + "IPPROTO_ICMPV6", + NaslValue::Number(IpNextHeaderProtocols::Icmpv6.to_primitive_values().0.into()), + ), ( "IPPROTO_IGMP", NaslValue::Number(IpNextHeaderProtocols::Igmp.to_primitive_values().0.into()), diff --git a/rust/src/nasl/builtin/tests.rs b/rust/src/nasl/builtin/tests.rs index 959fc8c0a0..b23e1c4a54 100644 --- a/rust/src/nasl/builtin/tests.rs +++ b/rust/src/nasl/builtin/tests.rs @@ -6,12 +6,37 @@ //! It would be nicer to have this within the proc_macro crate itself, //! but testing proc_macros comes with a lot of difficulties and the tests //! are very easy to do here. +use std::collections::HashMap; use crate::nasl::{ + builtin::misc::{ + NASL_ERR_ECONNRESET, NASL_ERR_ETIMEDOUT, NASL_ERR_EUNKNOWN, NASL_ERR_EUNREACH, + NASL_ERR_NOERR, + }, + nasl_std_executor, test_prelude::*, utils::{Executor, ScanCtx}, }; +#[test] +fn standard_executor_has_c_compatible_globals() { + let executor = nasl_std_executor(); + let globals = executor.iter_fn_global_vars().collect::>(); + let expected = [ + ("IPPROTO_ICMPV6", 58), + ("MSG_OOB", libc::MSG_OOB.into()), + ("NOERR", NASL_ERR_NOERR), + ("ETIMEDOUT", NASL_ERR_ETIMEDOUT), + ("ECONNRESET", NASL_ERR_ECONNRESET), + ("EUNREACH", NASL_ERR_EUNREACH), + ("EUNKNOWN", NASL_ERR_EUNKNOWN), + ]; + + for (name, value) in expected { + assert_eq!(globals.get(name), Some(&NaslValue::Number(value)), "{name}"); + } +} + #[nasl_function] fn foo1(_ctx: &ScanCtx, x: usize) -> usize { x diff --git a/rust/src/nasl/syntax/grammar.rs b/rust/src/nasl/syntax/grammar.rs index c6419b78ea..4739973659 100644 --- a/rust/src/nasl/syntax/grammar.rs +++ b/rust/src/nasl/syntax/grammar.rs @@ -6,7 +6,7 @@ use super::{ }; use crate::nasl::{ error::{Span, Spanned}, - syntax::token::{Ident, Literal, TokenKind}, + syntax::token::{Ident, Literal, LiteralKind, TokenKind}, }; #[derive(Clone, Debug)] @@ -219,6 +219,18 @@ pub enum Atom { Increment(Increment), } +impl Atom { + pub fn as_string_literal(&self) -> Option<&str> { + match self { + Self::Literal(Literal { + kind: LiteralKind::String(value), + .. + }) => Some(value), + _ => None, + } + } +} + #[derive(Clone, Debug)] pub struct Array { pub items: CommaSeparated, diff --git a/rust/src/nasl/syntax/mod.rs b/rust/src/nasl/syntax/mod.rs index ddad766a96..c66d82aca3 100644 --- a/rust/src/nasl/syntax/mod.rs +++ b/rust/src/nasl/syntax/mod.rs @@ -21,4 +21,4 @@ pub use token::{Ident, Token}; pub use tokenizer::CharIndex; pub use tokenizer::Tokenizer; pub use tokenizer::TokenizerError; -pub use visitor::{Visitor, walk_ast}; +pub use visitor::{Visitor, walk_ast, walk_block}; diff --git a/rust/src/nasl/syntax/visitor.rs b/rust/src/nasl/syntax/visitor.rs index 3b247d965e..8edf520650 100644 --- a/rust/src/nasl/syntax/visitor.rs +++ b/rust/src/nasl/syntax/visitor.rs @@ -7,10 +7,14 @@ pub trait Visitor<'ast> { fn visit_statement(&mut self, _stmt: &'ast Statement) {} fn visit_var_scope_decl(&mut self, _decl: &'ast VarScopeDecl) {} fn visit_fn_decl(&mut self, _decl: &'ast FnDecl) {} + fn should_walk_fn_body(&self, _decl: &'ast FnDecl) -> bool { + true + } fn visit_block(&mut self, _block: &'ast Block) {} fn visit_while(&mut self, _while_stmt: &'ast While) {} fn visit_repeat(&mut self, _repeat: &'ast Repeat) {} fn visit_for_each(&mut self, _for_each: &'ast ForEach) {} + fn visit_for_each_binding(&mut self, _for_each: &'ast ForEach) {} fn visit_for(&mut self, _for_stmt: &'ast For) {} fn visit_if(&mut self, _if_stmt: &'ast If) {} fn visit_include(&mut self, _include: &'ast Include) {} @@ -22,13 +26,16 @@ pub trait Visitor<'ast> { fn visit_binary(&mut self, _binary: &'ast Binary) {} fn visit_unary(&mut self, _unary: &'ast Unary) {} fn visit_assignment(&mut self, _assignment: &'ast Assignment) {} + fn leave_assignment(&mut self, _assignment: &'ast Assignment) {} // Atom visitors fn visit_atom(&mut self, _atom: &'ast Atom) {} fn visit_array(&mut self, _array: &'ast Array) {} fn visit_array_access(&mut self, _access: &'ast ArrayAccess) {} fn visit_fn_call(&mut self, _call: &'ast FnCall) {} + fn leave_fn_call(&mut self, _call: &'ast FnCall) {} fn visit_increment(&mut self, _inc: &'ast Increment) {} + fn leave_increment(&mut self, _inc: &'ast Increment) {} fn visit_literal(&mut self, _literal: &'ast super::super::syntax::token::Literal) {} fn visit_ident(&mut self, _ident: &'ast super::super::syntax::token::Ident) {} @@ -63,7 +70,7 @@ fn walk_statement<'ast, V: Visitor<'ast>>(visitor: &mut V, stmt: &'ast Statement } } -fn walk_block<'ast, V: Visitor<'ast>>(visitor: &mut V, block: &'ast Block) { +pub fn walk_block<'ast, V: Visitor<'ast>>(visitor: &mut V, block: &'ast Block) { visitor.visit_block(block); for stmt in &block.items { walk_statement(visitor, stmt); @@ -86,6 +93,7 @@ fn walk_for_each<'ast, V: Visitor<'ast>>(visitor: &mut V, for_each: &'ast ForEac visitor.visit_for_each(for_each); visitor.visit_ident(&for_each.var); walk_expr(visitor, &for_each.array); + visitor.visit_for_each_binding(for_each); walk_block(visitor, &for_each.block); } @@ -130,7 +138,9 @@ fn walk_fn_decl<'ast, V: Visitor<'ast>>(visitor: &mut V, fn_decl: &'ast FnDecl) for arg in &fn_decl.args.items { visitor.visit_ident(arg); } - walk_block(visitor, &fn_decl.block); + if visitor.should_walk_fn_body(fn_decl) { + walk_block(visitor, &fn_decl.block); + } } fn walk_var_scope_decl<'ast, V: Visitor<'ast>>(visitor: &mut V, var_decl: &'ast VarScopeDecl) { @@ -169,6 +179,7 @@ fn walk_assignment<'ast, V: Visitor<'ast>>(visitor: &mut V, assignment: &'ast As visitor.visit_assignment(assignment); walk_place_expr(visitor, &assignment.lhs); walk_expr(visitor, &assignment.rhs); + visitor.leave_assignment(assignment); } fn walk_place_expr<'ast, V: Visitor<'ast>>(visitor: &mut V, place: &'ast PlaceExpr) { @@ -213,6 +224,7 @@ fn walk_fn_call<'ast, V: Visitor<'ast>>(visitor: &mut V, call: &'ast FnCall) { for arg in &call.args.items { walk_fn_arg(visitor, arg); } + visitor.leave_fn_call(call); } fn walk_fn_arg<'ast, V: Visitor<'ast>>(visitor: &mut V, arg: &'ast FnArg) { @@ -229,4 +241,5 @@ fn walk_fn_arg<'ast, V: Visitor<'ast>>(visitor: &mut V, arg: &'ast FnArg) { fn walk_increment<'ast, V: Visitor<'ast>>(visitor: &mut V, inc: &'ast Increment) { visitor.visit_increment(inc); walk_place_expr(visitor, &inc.expr); + visitor.leave_increment(inc); } diff --git a/rust/src/nasl/utils/executor/mod.rs b/rust/src/nasl/utils/executor/mod.rs index 1abe3f5f75..5fb20e2fc3 100644 --- a/rust/src/nasl/utils/executor/mod.rs +++ b/rust/src/nasl/utils/executor/mod.rs @@ -88,6 +88,11 @@ impl Executor { self.fn_global_vars.iter().cloned() } + /// Names of global variables supplied by registered builtin function sets. + pub fn iter_global_var_names(&self) -> impl Iterator { + self.fn_global_vars.iter().map(|(name, _)| *name) + } + pub fn iter(&self) -> impl Iterator { self.sets.iter().flat_map(|set| set.iter()) } diff --git a/rust/src/scannerctl/linter/cli.rs b/rust/src/scannerctl/linter/cli.rs index 2a0bb122db..ad9ebb02fc 100644 --- a/rust/src/scannerctl/linter/cli.rs +++ b/rust/src/scannerctl/linter/cli.rs @@ -5,7 +5,8 @@ use scannerlib::nasl::Loader; #[derive(clap::Parser)] pub struct LinterArgs { - /// Either a single NASL file or a directory of NASL files on which to run the linter. + /// Either a single NASL file or a feed directory. Directory scans use + /// `.nasl` files as roots and load `.inc` files through include statements. pub path: PathBuf, } @@ -17,11 +18,46 @@ pub(super) fn get_files_and_loader(root: &Path) -> Result<(Loader, Vec) } else { for e in walkdir::WalkDir::new(root) { let e = e.map_err(std::io::Error::from)?; - if let Some("nasl") | Some("inc") = e.path().extension().and_then(|ext| ext.to_str()) { + if e.path().extension().and_then(|ext| ext.to_str()) == Some("nasl") { files.push(e.path().strip_prefix(root).unwrap().to_owned()); } } + files.sort(); Loader::from_feed_path(root) }; Ok((loader, files)) } + +#[cfg(test)] +mod tests { + use std::fs; + + use super::*; + + #[test] + fn directory_uses_only_nasl_files_as_roots() { + let directory = tempfile::tempdir().unwrap(); + fs::write(directory.path().join("first.nasl"), "").unwrap(); + fs::write(directory.path().join("second.nasl"), "").unwrap(); + fs::write(directory.path().join("functions.inc"), "").unwrap(); + + let (_, mut files) = get_files_and_loader(directory.path()).unwrap(); + files.sort(); + + assert_eq!( + files, + vec![PathBuf::from("first.nasl"), PathBuf::from("second.nasl")] + ); + } + + #[test] + fn explicit_include_file_is_still_a_root() { + let directory = tempfile::tempdir().unwrap(); + let include = directory.path().join("functions.inc"); + fs::write(&include, "").unwrap(); + + let (_, files) = get_files_and_loader(&include).unwrap(); + + assert_eq!(files, vec![PathBuf::from("functions.inc")]); + } +} diff --git a/rust/src/scannerctl/linter/ctx.rs b/rust/src/scannerctl/linter/ctx.rs index 9b35b3ad7c..f7c8e047a1 100644 --- a/rust/src/scannerctl/linter/ctx.rs +++ b/rust/src/scannerctl/linter/ctx.rs @@ -1,75 +1,240 @@ -use std::collections::HashMap; +use std::{ + collections::{HashMap, HashSet}, + sync::Arc, +}; use scannerlib::nasl::{ - nasl_std_executor, + SourceFile, nasl_std_executor, syntax::{ Visitor, grammar::{Ast, FnDecl}, walk_ast, }, + utils::Executor, }; -#[derive(Default)] +use super::paths::{IncludePath, ResolvedPath}; + +/// Names supplied by the script execution context or accepted for C-linter compatibility, +/// rather than registered as globals by Rust builtin function sets. +const PREDEFINED_VARS: [&str; 5] = [ + "ACT_UNKNOWN", + "COMMAND_LINE", + "OPENVAS_VERSION", + "SCRIPT_NAME", + "description", +]; + +/// Functions registered by the C NASL interpreter but not by `nasl_std_executor`. +/// Keep this list synchronized with `libfuncs` in `nasl/nasl_init.c`. +const C_COMPAT_BUILTINS: [&str; 42] = [ + "dsa_do_sign", + "dsa_do_verify", + "exit", + "file_close", + "file_open", + "file_read", + "file_seek", + "file_write", + "get_host_kb_index", + "psrp_cli", + "script_get_preference_file_location", + "smb_close", + "smb_connect", + "smb_file_SDDL", + "smb_file_group_sid", + "smb_file_owner_sid", + "smb_file_trustee_rights", + "socket_check_ssl_safe_renegotiation", + "socket_ssl_do_handshake", + "tls1_prf", + "update_table_driven_lsc_data", + "win_cmd_exec", + "wmi_close", + "wmi_connect", + "wmi_connect_reg", + "wmi_connect_rsop", + "wmi_query", + "wmi_query_rsop", + "wmi_reg_create_key", + "wmi_reg_delete_key", + "wmi_reg_enum_key", + "wmi_reg_enum_value", + "wmi_reg_get_bin_val", + "wmi_reg_get_dword_val", + "wmi_reg_get_ex_string_val", + "wmi_reg_get_mul_string_val", + "wmi_reg_get_qword_val", + "wmi_reg_get_sz", + "wmi_reg_set_dword_val", + "wmi_reg_set_ex_string_val", + "wmi_reg_set_qword_val", + "wmi_reg_set_string_val", +]; + +fn predefined_vars(executor: &Executor) -> HashSet { + executor + .iter_global_var_names() + .chain(PREDEFINED_VARS) + .map(str::to_owned) + .collect() +} + +fn builtin_fns(executor: &Executor) -> HashSet { + executor + .iter() + .chain(C_COMPAT_BUILTINS) + .map(str::to_owned) + .collect() +} + pub(crate) struct CachedFile { + ast: Ast, + file: SourceFile, fns: HashMap, } impl CachedFile { - pub(crate) fn new(ast: &Ast) -> Self { + pub(crate) fn new(file: SourceFile, ast: &Ast) -> Self { let mut collector = FnDefinitionCollector::default(); walk_ast(&mut collector, ast); CachedFile { + ast: ast.clone(), + file, fns: collector.functions, } } -} -pub struct BuiltinFn; + pub(crate) fn ast(&self) -> &Ast { + &self.ast + } + + pub(crate) fn file(&self) -> &SourceFile { + &self.file + } + + pub(crate) fn functions(&self) -> impl Iterator { + self.fns + .iter() + .map(|(name, declaration)| (name.as_str(), declaration)) + } +} pub(crate) struct Cache { - files: HashMap, - builtin_fns: HashMap, + files: HashMap>, + /// Resolves an include as written in a parent AST to its loader path. + include_paths: HashMap, + builtin_fns: HashSet, + predefined_vars: HashSet, +} + +#[derive(Eq, Hash, PartialEq)] +struct IncludeKey { + parent_path: ResolvedPath, + include_path: IncludePath, +} + +impl IncludeKey { + fn new(parent_path: &ResolvedPath, include_path: &IncludePath) -> Self { + Self { + parent_path: parent_path.clone(), + include_path: include_path.clone(), + } + } } impl Default for Cache { fn default() -> Self { - let builtin_fns = nasl_std_executor() - .iter() - .map(|name| (name.to_owned(), BuiltinFn)) - .collect(); + let executor = nasl_std_executor(); + let builtin_fns = builtin_fns(&executor); + let predefined_vars = predefined_vars(&executor); Self { files: HashMap::new(), + include_paths: HashMap::new(), builtin_fns, + predefined_vars, } } } impl Cache { - pub(crate) fn insert(&mut self, rel_path: &str, file: CachedFile) { - self.files.insert(rel_path.to_owned(), file); + pub(crate) fn clear_files(&mut self) { + self.files.clear(); + self.include_paths.clear(); + } + + pub(crate) fn insert(&mut self, path: &ResolvedPath, file: Arc) { + self.files.insert(path.clone(), file); + } + + pub(crate) fn files(&self) -> impl Iterator { + self.files.iter().map(|(path, file)| (path, file.as_ref())) + } + + pub(crate) fn file(&self, path: &ResolvedPath) -> Option<&CachedFile> { + self.files.get(path).map(Arc::as_ref) + } + + pub(crate) fn record_include( + &mut self, + parent_path: &ResolvedPath, + include_path: &IncludePath, + resolved_path: &ResolvedPath, + ) { + self.include_paths.insert( + IncludeKey::new(parent_path, include_path), + resolved_path.clone(), + ); + } + + pub(crate) fn included_path( + &self, + parent_path: &ResolvedPath, + include_path: &IncludePath, + ) -> Option<&ResolvedPath> { + self.include_paths + .get(&IncludeKey::new(parent_path, include_path)) + } + + pub(crate) fn included_file( + &self, + parent_path: &ResolvedPath, + include_path: &IncludePath, + ) -> Option<(&ResolvedPath, &CachedFile)> { + let path = self.included_path(parent_path, include_path)?; + self.file(path).map(|file| (path, file)) + } + + pub(crate) fn predefined_vars(&self) -> impl Iterator { + self.predefined_vars.iter().map(String::as_str) } } pub(crate) struct LintCtx<'a> { pub cache: &'a mut Cache, pub ast: &'a Ast, + pub file: &'a SourceFile, + pub path: &'a ResolvedPath, } impl<'a> LintCtx<'a> { - pub fn new(ast: &'a Ast, cache: &'a mut Cache) -> Self { - Self { cache, ast } - } - - pub fn fn_defined(&self, fn_name: &str) -> bool { - self.cache - .files - .values() - .any(|file| file.fns.contains_key(fn_name)) + pub fn new( + ast: &'a Ast, + file: &'a SourceFile, + path: &'a ResolvedPath, + cache: &'a mut Cache, + ) -> Self { + Self { + cache, + ast, + file, + path, + } } pub(crate) fn builtin_defined(&self, fn_name: &str) -> bool { - self.cache.builtin_fns.contains_key(fn_name) + self.cache.builtin_fns.contains(fn_name) } } @@ -81,6 +246,26 @@ pub(crate) struct FnDefinitionCollector { impl<'ast> Visitor<'ast> for FnDefinitionCollector { fn visit_fn_decl(&mut self, decl: &'ast FnDecl) { let fn_name = decl.fn_name.to_string(); - self.functions.insert(fn_name, decl.clone()); + self.functions + .entry(fn_name) + .or_insert_with(|| decl.clone()); + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn c_compat_builtins_are_not_in_rust_executor() { + let executor = nasl_std_executor(); + let executor_builtins = executor.iter().collect::>(); + + for name in C_COMPAT_BUILTINS { + assert!( + !executor_builtins.contains(name), + "C-compatible builtin `{name}` is already registered by the Rust executor" + ); + } } } diff --git a/rust/src/scannerctl/linter/lints/duplicate_function_arg.rs b/rust/src/scannerctl/linter/lints/duplicate_function_arg.rs index 34fa8b8330..605dc41eaa 100644 --- a/rust/src/scannerctl/linter/lints/duplicate_function_arg.rs +++ b/rust/src/scannerctl/linter/lints/duplicate_function_arg.rs @@ -2,11 +2,14 @@ use std::collections::HashMap; use codespan_reporting::diagnostic::{Diagnostic, Label}; use scannerlib::nasl::{ + SourceFile, error::{Span, Spanned}, - syntax::grammar::{Ast, FnArg, FnCall}, + syntax::grammar::{FnArg, FnCall}, }; -use crate::linter::LintMsg; +use crate::linter::{LintMsg, ctx::LintCtx}; + +const RULE: &str = "duplicate_function_argument"; struct Entry { count: usize, @@ -34,7 +37,7 @@ impl Entry { } } -pub fn get_duplicate_args(fn_call: &FnCall) -> Vec { +fn get_duplicate_args(file: &SourceFile, fn_call: &FnCall) -> Vec { let mut counter: HashMap<_, _> = HashMap::default(); for arg in fn_call.args.items.iter() { if let FnArg::Named(arg) = arg { @@ -49,12 +52,26 @@ pub fn get_duplicate_args(fn_call: &FnCall) -> Vec { counter .into_iter() .filter(|(_, entry)| entry.count > 1) - .map(|(name, entry)| entry.into_diagnostic(&name).into()) + .map(|(name, entry)| { + let span = entry.spans[0]; + let diagnostic = entry.into_diagnostic(&name); + LintMsg::new(RULE, file.clone(), span, diagnostic) + }) .collect() } -pub fn duplicate_function_args(ast: &Ast) -> Vec { - ast.iter_fn_calls().flat_map(get_duplicate_args).collect() +pub fn duplicate_function_args(ctx: &LintCtx) -> Vec { + let mut files = ctx.cache.files().collect::>(); + files.sort_by(|(left, _), (right, _)| left.cmp(right)); + + files + .into_iter() + .flat_map(|(_, file)| { + file.ast() + .iter_fn_calls() + .flat_map(|fn_call| get_duplicate_args(file.file(), fn_call)) + }) + .collect() } #[cfg(test)] diff --git a/rust/src/scannerctl/linter/lints/duplicate_function_declaration.rs b/rust/src/scannerctl/linter/lints/duplicate_function_declaration.rs new file mode 100644 index 0000000000..a43fa6a1f5 --- /dev/null +++ b/rust/src/scannerctl/linter/lints/duplicate_function_declaration.rs @@ -0,0 +1,158 @@ +use std::collections::HashMap; + +use codespan_reporting::diagnostic::{Diagnostic, Label}; +use scannerlib::nasl::{ + error::{Span, Spanned}, + syntax::grammar::Statement, +}; + +use crate::linter::paths::ResolvedPath; +use crate::linter::{LintMsg, ctx::LintCtx}; + +const RULE: &str = "duplicate_function_declaration"; + +struct Declarations { + name: String, + spans: Vec, +} + +impl Declarations { + fn into_diagnostic(self) -> Diagnostic<()> { + let message = format!("Function declared multiple times: {}", self.name); + let labels = self + .spans + .into_iter() + .enumerate() + .map(|(index, span)| { + if index == 0 { + Label::primary((), span).with_message("first declaration") + } else { + Label::secondary((), span).with_message("redeclared here") + } + }) + .collect(); + Diagnostic::error() + .with_message(message) + .with_labels(labels) + } +} + +fn duplicate_function_declarations_in_file(ctx: &LintCtx) -> Vec { + let mut declarations = Vec::::new(); + let mut indices = HashMap::::new(); + + for declaration in ctx + .ast + .iter_stmts() + .filter_map(|statement| match statement { + Statement::FnDecl(declaration) => Some(declaration), + _ => None, + }) + { + let name = declaration.fn_name.to_string(); + if let Some(index) = indices.get(&name) { + declarations[*index].spans.push(declaration.fn_name.span()); + } else { + indices.insert(name.clone(), declarations.len()); + declarations.push(Declarations { + name, + spans: vec![declaration.fn_name.span()], + }); + } + } + + declarations + .into_iter() + .filter(|declarations| declarations.spans.len() > 1) + .map(|declarations| { + let span = declarations.spans[0]; + let diagnostic = declarations.into_diagnostic(); + LintMsg::new(RULE, ctx.file.clone(), span, diagnostic) + }) + .collect() +} + +fn duplicate_function_declarations_across_files(ctx: &LintCtx) -> Vec { + let mut first_declarations = HashMap::::new(); + let mut messages = vec![]; + + let mut files = ctx.cache.files().collect::>(); + files.sort_by(|(left, _), (right, _)| left.cmp(right)); + + for (path, file) in files { + let mut functions = file.functions().collect::>(); + functions.sort_by_key(|(_, declaration)| { + let span: std::ops::Range = declaration.fn_name.span().into(); + span.start + }); + + for (name, declaration) in functions { + if let Some(first_file) = first_declarations.get(name) { + if first_file == path { + continue; + } + + let message = format!("Function declared multiple times: {name}"); + let span = declaration.fn_name.span(); + let diagnostic = Diagnostic::error() + .with_message(message) + .with_labels(vec![ + Label::primary((), span).with_message("redeclared here"), + ]) + .with_notes(vec![format!("also declared in {first_file}")]); + messages.push(LintMsg::new(RULE, file.file().clone(), span, diagnostic)); + } else { + first_declarations.insert(name.to_owned(), path.clone()); + } + } + } + + messages +} + +fn builtin_function_redefinitions(ctx: &LintCtx) -> Vec { + let mut files = ctx.cache.files().collect::>(); + files.sort_by(|(left, _), (right, _)| left.cmp(right)); + + files + .into_iter() + .flat_map(|(_, file)| { + let mut functions = file + .functions() + .filter(|(name, _)| ctx.builtin_defined(name)) + .collect::>(); + functions.sort_by_key(|(_, declaration)| { + let span: std::ops::Range = declaration.fn_name.span().into(); + span.start + }); + + functions.into_iter().map(move |(name, declaration)| { + let span = declaration.fn_name.span(); + let diagnostic = Diagnostic::error() + .with_message(format!("Cannot redefine built-in function: {name}")) + .with_labels(vec![ + Label::primary((), span).with_message("built-in function redefined here"), + ]); + LintMsg::new(RULE, file.file().clone(), span, diagnostic) + }) + }) + .collect() +} + +pub fn duplicate_function_declarations(ctx: &LintCtx) -> Vec { + duplicate_function_declarations_in_file(ctx) + .into_iter() + .chain(duplicate_function_declarations_across_files(ctx)) + .chain(builtin_function_redefinitions(ctx)) + .collect() +} + +#[cfg(test)] +mod tests { + use crate::linter_test; + + linter_test!(duplicate_function, "function foo() {} function foo() {}"); + linter_test!(distinct_functions, "function foo() {} function bar() {}"); + linter_test!(builtin_function, "function display() {}"); + linter_test!(c_compat_builtin_function, "function wmi_query() {}"); +} diff --git a/rust/src/scannerctl/linter/lints/fn_undefined.rs b/rust/src/scannerctl/linter/lints/fn_undefined.rs index eeb5b60f3c..ef4a958332 100644 --- a/rust/src/scannerctl/linter/lints/fn_undefined.rs +++ b/rust/src/scannerctl/linter/lints/fn_undefined.rs @@ -1,31 +1,226 @@ +use std::{ + collections::{HashMap, HashSet}, + mem, + ops::Range, +}; + use codespan_reporting::diagnostic::{Diagnostic, Label}; -use scannerlib::nasl::error::Spanned; +use scannerlib::nasl::{ + error::{Span, Spanned}, + syntax::{ + Visitor, + grammar::{FnCall, FnDecl, Include}, + walk_ast, walk_block, + }, +}; + +use crate::linter::{ + LintMsg, + ctx::{Cache, LintCtx}, + paths::{IncludePath, ResolvedPath}, +}; + +const RULE: &str = "undefined_function"; +// This a very ugly implementation detail: openvas-nasl-lint would +// explicitly exclude undefined functions of the structure +// `if defined_func("foo") { foo() }`. I find this to be a very ugly +// concept, however it does take care of a few scripts in the feed where +// this would otherwise result in false positives. +// Here, I am explicitly including those exceptions. +// This is ugly too, but makes it a little bit easier to remove this +// in case we can convince feed authors to remove the obsolete functions. +const UNDEFINED_FUNCTION_EXCEPTIONS: [&str; 2] = ["network_targets", "scan_phase"]; + +struct FunctionDefinition { + path: ResolvedPath, + declaration: FnDecl, +} + +struct CallSite { + path: ResolvedPath, + name: String, + span: Span, +} + +struct ReachableCalls<'cache> { + cache: &'cache Cache, + path: ResolvedPath, + loaded: HashSet, + calls: Vec, +} + +impl<'cache> ReachableCalls<'cache> { + fn new(cache: &'cache Cache, path: ResolvedPath) -> Self { + let loaded = HashSet::from([path.clone()]); + Self { + cache, + path, + loaded, + calls: vec![], + } + } + + fn walk_function(&mut self, definition: &FunctionDefinition) { + let outer_path = mem::replace(&mut self.path, definition.path.clone()); + walk_block(self, &definition.declaration.block); + self.path = outer_path; + } + + fn walk_include(&mut self, include: &Include) { + let include_path = IncludePath::new(include.path.as_str()); + let Some((path, ast)) = self + .cache + .included_file(&self.path, &include_path) + .map(|(path, file)| (path.clone(), file.ast().clone())) + else { + return; + }; + if !self.loaded.insert(path.clone()) { + return; + } -use crate::linter::{LintMsg, ctx::LintCtx}; + let outer_path = mem::replace(&mut self.path, path); + walk_ast(self, &ast); + self.path = outer_path; + } +} + +impl<'ast> Visitor<'ast> for ReachableCalls<'_> { + fn visit_fn_call(&mut self, call: &'ast FnCall) { + self.calls.push(CallSite { + path: self.path.clone(), + name: call.fn_name.to_string(), + span: call.fn_name.span(), + }); + } + + fn visit_include(&mut self, include: &'ast Include) { + self.walk_include(include); + } + + fn should_walk_fn_body(&self, _declaration: &'ast FnDecl) -> bool { + false + } +} + +fn function_definitions(cache: &Cache) -> HashMap> { + let mut definitions: HashMap> = HashMap::new(); + for (path, file) in cache.files() { + for (name, declaration) in file.functions() { + definitions + .entry(name.to_owned()) + .or_default() + .push(FunctionDefinition { + path: path.clone(), + declaration: declaration.clone(), + }); + } + } + definitions +} pub fn fn_undefined(ctx: &LintCtx) -> Vec { - ctx.ast - .iter_fn_calls() - .filter(|call| { - let name = call.fn_name.to_string(); - !ctx.fn_defined(&name) && !ctx.builtin_defined(&name) + let cache = &*ctx.cache; + let definitions = function_definitions(cache); + let mut reachable = ReachableCalls::new(cache, ctx.path.clone()); + walk_ast(&mut reachable, ctx.ast); + + // Here, we build up a collection of all the called functions by + // starting with the top level function calls and then adding + // calls from each functions body to a queue. + let mut reachable_functions = HashSet::new(); + // We do this instead of iterating normally to make borrowck happy + let mut next_call = 0; + while let Some(name) = reachable + .calls + .get(next_call) + .map(|call_site| call_site.name.clone()) + { + next_call += 1; + if ctx.builtin_defined(&name) || !reachable_functions.insert(name.clone()) { + continue; + } + if let Some(definitions) = definitions.get(&name) { + for definition in definitions { + reachable.walk_function(definition); + } + } + } + + let mut undefined = reachable + .calls + .into_iter() + .filter(|call_site| { + !definitions.contains_key(&call_site.name) + && !ctx.builtin_defined(&call_site.name) + && !UNDEFINED_FUNCTION_EXCEPTIONS.contains(&call_site.name.as_str()) }) - .map(|call| { - Diagnostic::error() - .with_message(format!("Undefined function '{}'", call.fn_name)) + .collect::>(); + undefined.sort_by(|left, right| { + let left_span: Range = left.span.into(); + let right_span: Range = right.span.into(); + left.path + .cmp(&right.path) + .then(left_span.start.cmp(&right_span.start)) + }); + + undefined + .into_iter() + .filter_map(|call_site| { + let file = cache.file(&call_site.path)?.file().clone(); + let diagnostic = Diagnostic::error() + .with_message(format!("Undefined function '{}'", call_site.name)) .with_labels(vec![ - Label::primary((), call.fn_name.span()).with_message("undefined function"), - ]) - .into() + Label::primary((), call_site.span).with_message("undefined function"), + ]); + Some(LintMsg::new(RULE, file, call_site.span, diagnostic)) }) .collect() } #[cfg(test)] mod tests { - use crate::linter_test; + use crate::{linter_test, linter_test_multi}; linter_test!(undefined_fn, "foo();"); linter_test!(defined_fn, "function foo() {} foo();"); linter_test!(builtin_fn, "display(\"hello\");"); + linter_test!( + unreachable_root_function, + "function unused() { missing(); }" + ); + linter_test!( + transitively_reachable_function, + "function first() { second(); } function second() { missing(); } first();" + ); + linter_test!( + defined_func_does_not_declare_function, + "if (defined_func(\"optional\")) { optional(); }" + ); + linter_test!( + undefined_function_exceptions, + "scan_phase(); network_targets();" + ); + linter_test!( + c_compat_builtin_fns, + "wmi_query(); socket_ssl_do_handshake();" + ); + + linter_test_multi!( + unreachable_include_function, + roots: ["root.nasl"], + files: { + "root.nasl" => "include(\"library.inc\"); used();", + "library.inc" => "function used() {} function unused() { missing(); }", + }, + ); + + linter_test_multi!( + reachable_include_function, + roots: ["root.nasl"], + files: { + "root.nasl" => "include(\"library.inc\"); used();", + "library.inc" => "function used() { missing(); }", + }, + ); } diff --git a/rust/src/scannerctl/linter/lints/mod.rs b/rust/src/scannerctl/linter/lints/mod.rs index 74f317fdc0..8d05e23ca8 100644 --- a/rust/src/scannerctl/linter/lints/mod.rs +++ b/rust/src/scannerctl/linter/lints/mod.rs @@ -1,19 +1,63 @@ mod duplicate_function_arg; +mod duplicate_function_declaration; mod fn_undefined; +mod script_xref; +mod undeclared_variable; +mod unused_include; + +use std::ops::Range; use codespan_reporting::diagnostic::Diagnostic; -use scannerlib::nasl::error::IntoDiagnostic; -use scannerlib::nasl::syntax::grammar::Ast; +use scannerlib::nasl::{ + SourceFile, + error::{IntoDiagnostic, Span}, +}; use super::ctx::LintCtx; +/// A key used to identify the same lint message when it is created +/// multiple times. Getting the same message multiple times happens when +/// a file is included from multiple places. +#[derive(Clone, Debug, Eq, Hash, PartialEq)] +pub(super) struct LintMsgKey { + rule: &'static str, + file: String, + span: Range, +} + +#[derive(Clone)] pub(super) struct LintMsg { + rule: &'static str, + file: SourceFile, + span: Range, diagnostic: Diagnostic<()>, } -impl From> for LintMsg { - fn from(diagnostic: Diagnostic<()>) -> Self { - Self { diagnostic } +impl LintMsg { + pub(super) fn new( + rule: &'static str, + file: SourceFile, + span: Span, + diagnostic: Diagnostic<()>, + ) -> Self { + Self { + rule, + file, + span: span.into(), + diagnostic, + } + } + + pub(super) fn file(&self) -> &SourceFile { + &self.file + } + + pub(super) fn message_key(&self) -> LintMsgKey { + LintMsgKey { + rule: self.rule, + file: self.file.name().clone(), + span: self.span.clone(), + } } } @@ -27,17 +71,6 @@ pub(super) trait Lint { fn lint<'a>(&self, ctx: &LintCtx<'a>) -> Vec; } -struct AstLint(T); - -impl Lint for AstLint -where - T: Fn(&Ast) -> Vec, -{ - fn lint<'a>(&self, ctx: &LintCtx<'a>) -> Vec { - (self.0)(ctx.ast) - } -} - struct FnLint(T); impl Lint for FnLint @@ -50,10 +83,13 @@ where } pub fn all_lints() -> Vec> { - let ast_lint = |f| Box::new(AstLint(f)) as Box; - let fn_lint = |f| Box::new(FnLint(f)) as Box; + let fn_lint = |f: fn(&LintCtx) -> Vec| Box::new(FnLint(f)) as Box; vec![ - ast_lint(duplicate_function_arg::duplicate_function_args), + fn_lint(duplicate_function_arg::duplicate_function_args), + fn_lint(duplicate_function_declaration::duplicate_function_declarations), fn_lint(fn_undefined::fn_undefined), + fn_lint(script_xref::script_xref), + fn_lint(undeclared_variable::undeclared_variables), + fn_lint(unused_include::unused_includes), ] } diff --git a/rust/src/scannerctl/linter/lints/script_xref.rs b/rust/src/scannerctl/linter/lints/script_xref.rs new file mode 100644 index 0000000000..0a7a639cae --- /dev/null +++ b/rust/src/scannerctl/linter/lints/script_xref.rs @@ -0,0 +1,159 @@ +use codespan_reporting::diagnostic::{Diagnostic, Label}; +use scannerlib::nasl::{ + SourceFile, + error::{Span, Spanned}, + syntax::{ + Visitor, + grammar::{Atom, FnArg, FnCall}, + walk_ast, + }, +}; + +use crate::linter::{LintMsg, ctx::LintCtx}; + +const RULE: &str = "script_xref"; + +struct Call { + span: Span, + missing_name: bool, + missing_reference: bool, + invalid_string_spans: Vec, +} + +impl Call { + fn new(call: &FnCall) -> Self { + let mut has_name = false; + let mut has_value = false; + let mut has_csv = false; + + for argument in &call.args.items { + let FnArg::Named(argument) = argument else { + continue; + }; + match argument.ident.to_string().as_str() { + "name" => has_name = true, + "value" => has_value = true, + "csv" => has_csv = true, + _ => {} + } + } + + Self { + span: call.fn_name.span(), + missing_name: !has_name, + missing_reference: !has_value && !has_csv, + invalid_string_spans: vec![], + } + } + + fn is_valid(&self) -> bool { + !self.missing_name && !self.missing_reference && self.invalid_string_spans.is_empty() + } + + fn into_message(self, file: SourceFile) -> Option { + if self.is_valid() { + return None; + } + + let mut labels = vec![]; + let missing_message = match (self.missing_name, self.missing_reference) { + (true, true) => Some("`name` and at least one of `value` or `csv` are required"), + (true, false) => Some("`name` is required"), + (false, true) => Some("at least one of `value` or `csv` is required"), + (false, false) => None, + }; + if let Some(message) = missing_message { + labels.push(Label::primary((), self.span).with_message(message)); + } + labels.extend(self.invalid_string_spans.into_iter().map(|span| { + Label::primary((), span).with_message("xref strings must not contain `, `") + })); + + let diagnostic = Diagnostic::error() + .with_message("Invalid script_xref call") + .with_labels(labels); + Some(LintMsg::new(RULE, file, self.span, diagnostic)) + } +} + +#[derive(Default)] +struct ScriptXrefVisitor { + active_calls: Vec, + calls: Vec, +} + +impl<'ast> Visitor<'ast> for ScriptXrefVisitor { + fn visit_fn_call(&mut self, call: &'ast FnCall) { + if call.fn_name.to_string() == "script_xref" { + self.active_calls.push(Call::new(call)); + } + } + + fn leave_fn_call(&mut self, call: &'ast FnCall) { + if call.fn_name.to_string() == "script_xref" { + self.calls.push(self.active_calls.pop().unwrap()); + } + } + + fn visit_atom(&mut self, atom: &'ast Atom) { + if atom + .as_string_literal() + .is_some_and(|value| value.contains(", ")) + && let Some(call) = self.active_calls.last_mut() + { + call.invalid_string_spans.push(atom.span()); + } + } +} + +pub fn script_xref(ctx: &LintCtx) -> Vec { + let mut files = ctx.cache.files().collect::>(); + files.sort_by(|(left, _), (right, _)| left.cmp(right)); + + files + .into_iter() + .flat_map(|(_, file)| { + let mut visitor = ScriptXrefVisitor::default(); + walk_ast(&mut visitor, file.ast()); + visitor + .calls + .into_iter() + .filter_map(|call| call.into_message(file.file().clone())) + }) + .collect() +} + +#[cfg(test)] +mod tests { + use crate::{linter_test, linter_test_multi}; + + linter_test!( + valid_script_xrefs, + r#" +script_xref(name: "URL", value: "https://example.com"); +script_xref(name: "CVE", csv: "CVE-2025-0001,CVE-2025-0002"); +script_xref(name: "GHSA", value: "GHSA-3333-4444-5555", csv: "GHSA-1111-2222-3333,GHSA-2222-3333-4444"); +"# + ); + + linter_test!( + malformed_script_xrefs, + r#" +script_xref(value: "https://example.com"); +script_xref(name: "URL"); +script_xref(); +script_xref(name: "URL", value: "https://example.com/a, b"); +script_xref(name: "CVE, GHSA", csv: "CVE-2025-0001, CVE-2025-0002"); +"# + ); + + linter_test_multi!( + malformed_script_xref_in_shared_include_is_emitted_once, + roots: ["a.nasl", "b.nasl"], + files: { + "a.nasl" => "include(\"xref.inc\");", + "b.nasl" => "include(\"xref.inc\");", + "xref.inc" => "script_xref(name: \"URL\");", + }, + ); +} diff --git a/rust/src/scannerctl/linter/lints/snapshots/scannerctl__linter__lints__duplicate_function_declaration__tests__builtin_function.snap b/rust/src/scannerctl/linter/lints/snapshots/scannerctl__linter__lints__duplicate_function_declaration__tests__builtin_function.snap new file mode 100644 index 0000000000..a0e542b5d9 --- /dev/null +++ b/rust/src/scannerctl/linter/lints/snapshots/scannerctl__linter__lints__duplicate_function_declaration__tests__builtin_function.snap @@ -0,0 +1,10 @@ +--- +source: src/scannerctl/linter/lints/duplicate_function_declaration.rs +assertion_line: 149 +expression: "$crate :: linter :: tests ::\nlint(stringify! (builtin_function), \"function display() {}\",)" +--- +error: Cannot redefine built-in function: display + ┌─ builtin_function:1:10 + │ +1 │ function display() {} + │ ^^^^^^^ built-in function redefined here diff --git a/rust/src/scannerctl/linter/lints/snapshots/scannerctl__linter__lints__duplicate_function_declaration__tests__c_compat_builtin_function.snap b/rust/src/scannerctl/linter/lints/snapshots/scannerctl__linter__lints__duplicate_function_declaration__tests__c_compat_builtin_function.snap new file mode 100644 index 0000000000..4a6be767f5 --- /dev/null +++ b/rust/src/scannerctl/linter/lints/snapshots/scannerctl__linter__lints__duplicate_function_declaration__tests__c_compat_builtin_function.snap @@ -0,0 +1,10 @@ +--- +source: src/scannerctl/linter/lints/duplicate_function_declaration.rs +assertion_line: 156 +expression: "$crate :: linter :: tests ::\nlint(stringify! (c_compat_builtin_function), \"function wmi_query() {}\",)" +--- +error: Cannot redefine built-in function: wmi_query + ┌─ c_compat_builtin_function:1:10 + │ +1 │ function wmi_query() {} + │ ^^^^^^^^^ built-in function redefined here diff --git a/rust/src/scannerctl/linter/lints/snapshots/scannerctl__linter__lints__duplicate_function_declaration__tests__distinct_functions.snap b/rust/src/scannerctl/linter/lints/snapshots/scannerctl__linter__lints__duplicate_function_declaration__tests__distinct_functions.snap new file mode 100644 index 0000000000..7d50fa1323 --- /dev/null +++ b/rust/src/scannerctl/linter/lints/snapshots/scannerctl__linter__lints__duplicate_function_declaration__tests__distinct_functions.snap @@ -0,0 +1,4 @@ +--- +source: src/scannerctl/linter/lints/duplicate_function_declaration.rs +expression: "$crate :: linter :: tests ::\nlint(stringify! (distinct_functions), \"function foo() {} function bar() {}\",)" +--- diff --git a/rust/src/scannerctl/linter/lints/snapshots/scannerctl__linter__lints__duplicate_function_declaration__tests__duplicate_function.snap b/rust/src/scannerctl/linter/lints/snapshots/scannerctl__linter__lints__duplicate_function_declaration__tests__duplicate_function.snap new file mode 100644 index 0000000000..6899b9968f --- /dev/null +++ b/rust/src/scannerctl/linter/lints/snapshots/scannerctl__linter__lints__duplicate_function_declaration__tests__duplicate_function.snap @@ -0,0 +1,11 @@ +--- +source: src/scannerctl/linter/lints/duplicate_function_declaration.rs +expression: "$crate :: linter :: tests ::\nlint(stringify! (duplicate_function), \"function foo() {} function foo() {}\",)" +--- +error: Function declared multiple times: foo + ┌─ duplicate_function:1:10 + │ +1 │ function foo() {} function foo() {} + │ ^^^ --- redeclared here + │ │ + │ first declaration diff --git a/rust/src/scannerctl/linter/lints/snapshots/scannerctl__linter__lints__fn_undefined__tests__c_compat_builtin_fns.snap b/rust/src/scannerctl/linter/lints/snapshots/scannerctl__linter__lints__fn_undefined__tests__c_compat_builtin_fns.snap new file mode 100644 index 0000000000..436944a910 --- /dev/null +++ b/rust/src/scannerctl/linter/lints/snapshots/scannerctl__linter__lints__fn_undefined__tests__c_compat_builtin_fns.snap @@ -0,0 +1,5 @@ +--- +source: src/scannerctl/linter/lints/fn_undefined.rs +assertion_line: 41 +expression: "$crate :: linter :: tests ::\nlint(stringify! (c_compat_builtin_fns),\n\"wmi_query(); socket_ssl_do_handshake();\",)" +--- diff --git a/rust/src/scannerctl/linter/lints/snapshots/scannerctl__linter__lints__fn_undefined__tests__defined_func_does_not_declare_function.snap b/rust/src/scannerctl/linter/lints/snapshots/scannerctl__linter__lints__fn_undefined__tests__defined_func_does_not_declare_function.snap new file mode 100644 index 0000000000..7cc58c1647 --- /dev/null +++ b/rust/src/scannerctl/linter/lints/snapshots/scannerctl__linter__lints__fn_undefined__tests__defined_func_does_not_declare_function.snap @@ -0,0 +1,9 @@ +--- +source: src/scannerctl/linter/lints/fn_undefined.rs +expression: "$crate :: linter :: tests ::\nlint(stringify! (defined_func_does_not_declare_function),\n\"if (defined_func(\\\"optional\\\")) { optional(); }\",)" +--- +error: Undefined function 'optional' + ┌─ defined_func_does_not_declare_function:1:33 + │ +1 │ if (defined_func("optional")) { optional(); } + │ ^^^^^^^^ undefined function diff --git a/rust/src/scannerctl/linter/lints/snapshots/scannerctl__linter__lints__fn_undefined__tests__reachable_include_function.snap b/rust/src/scannerctl/linter/lints/snapshots/scannerctl__linter__lints__fn_undefined__tests__reachable_include_function.snap new file mode 100644 index 0000000000..23dddee93f --- /dev/null +++ b/rust/src/scannerctl/linter/lints/snapshots/scannerctl__linter__lints__fn_undefined__tests__reachable_include_function.snap @@ -0,0 +1,9 @@ +--- +source: src/scannerctl/linter/lints/fn_undefined.rs +expression: "$crate :: linter :: tests ::\nlint_files(& [\"root.nasl\"], &\n[(\"root.nasl\", \"include(\\\"library.inc\\\"); used();\"),\n(\"library.inc\", \"function used() { missing(); }\")],)" +--- +error: Undefined function 'missing' + ┌─ library.inc:1:19 + │ +1 │ function used() { missing(); } + │ ^^^^^^^ undefined function diff --git a/rust/src/scannerctl/linter/lints/snapshots/scannerctl__linter__lints__fn_undefined__tests__transitively_reachable_function.snap b/rust/src/scannerctl/linter/lints/snapshots/scannerctl__linter__lints__fn_undefined__tests__transitively_reachable_function.snap new file mode 100644 index 0000000000..a477fe0840 --- /dev/null +++ b/rust/src/scannerctl/linter/lints/snapshots/scannerctl__linter__lints__fn_undefined__tests__transitively_reachable_function.snap @@ -0,0 +1,9 @@ +--- +source: src/scannerctl/linter/lints/fn_undefined.rs +expression: "$crate :: linter :: tests ::\nlint(stringify! (transitively_reachable_function),\n\"function first() { second(); } function second() { missing(); } first();\",)" +--- +error: Undefined function 'missing' + ┌─ transitively_reachable_function:1:52 + │ +1 │ function first() { second(); } function second() { missing(); } first(); + │ ^^^^^^^ undefined function diff --git a/rust/src/scannerctl/linter/lints/snapshots/scannerctl__linter__lints__fn_undefined__tests__undefined_function_exceptions.snap b/rust/src/scannerctl/linter/lints/snapshots/scannerctl__linter__lints__fn_undefined__tests__undefined_function_exceptions.snap new file mode 100644 index 0000000000..917e8de851 --- /dev/null +++ b/rust/src/scannerctl/linter/lints/snapshots/scannerctl__linter__lints__fn_undefined__tests__undefined_function_exceptions.snap @@ -0,0 +1,4 @@ +--- +source: src/scannerctl/linter/lints/fn_undefined.rs +expression: "$crate :: linter :: tests ::\nlint(stringify! (undefined_function_exceptions),\n\"scan_phase(); network_targets();\",)" +--- diff --git a/rust/src/scannerctl/linter/lints/snapshots/scannerctl__linter__lints__fn_undefined__tests__unreachable_include_function.snap b/rust/src/scannerctl/linter/lints/snapshots/scannerctl__linter__lints__fn_undefined__tests__unreachable_include_function.snap new file mode 100644 index 0000000000..b7e153129d --- /dev/null +++ b/rust/src/scannerctl/linter/lints/snapshots/scannerctl__linter__lints__fn_undefined__tests__unreachable_include_function.snap @@ -0,0 +1,4 @@ +--- +source: src/scannerctl/linter/lints/fn_undefined.rs +expression: "$crate :: linter :: tests ::\nlint_files(& [\"root.nasl\"], &\n[(\"root.nasl\", \"include(\\\"library.inc\\\"); used();\"),\n(\"library.inc\", \"function used() {} function unused() { missing(); }\")],)" +--- diff --git a/rust/src/scannerctl/linter/lints/snapshots/scannerctl__linter__lints__fn_undefined__tests__unreachable_root_function.snap b/rust/src/scannerctl/linter/lints/snapshots/scannerctl__linter__lints__fn_undefined__tests__unreachable_root_function.snap new file mode 100644 index 0000000000..d0b2161d96 --- /dev/null +++ b/rust/src/scannerctl/linter/lints/snapshots/scannerctl__linter__lints__fn_undefined__tests__unreachable_root_function.snap @@ -0,0 +1,4 @@ +--- +source: src/scannerctl/linter/lints/fn_undefined.rs +expression: "$crate :: linter :: tests ::\nlint(stringify! (unreachable_root_function),\n\"function unused() { missing(); }\",)" +--- diff --git a/rust/src/scannerctl/linter/lints/snapshots/scannerctl__linter__lints__script_xref__tests__malformed_script_xref_in_shared_include_is_emitted_once.snap b/rust/src/scannerctl/linter/lints/snapshots/scannerctl__linter__lints__script_xref__tests__malformed_script_xref_in_shared_include_is_emitted_once.snap new file mode 100644 index 0000000000..0252eac2c0 --- /dev/null +++ b/rust/src/scannerctl/linter/lints/snapshots/scannerctl__linter__lints__script_xref__tests__malformed_script_xref_in_shared_include_is_emitted_once.snap @@ -0,0 +1,9 @@ +--- +source: src/scannerctl/linter/lints/script_xref.rs +expression: "$crate :: linter :: tests ::\nlint_files(& [\"a.nasl\", \"b.nasl\"], &\n[(\"a.nasl\", \"include(\\\"xref.inc\\\");\"), (\"b.nasl\", \"include(\\\"xref.inc\\\");\"),\n(\"xref.inc\", \"script_xref(name: \\\"URL\\\");\")],)" +--- +error: Invalid script_xref call + ┌─ xref.inc:1:1 + │ +1 │ script_xref(name: "URL"); + │ ^^^^^^^^^^^ at least one of `value` or `csv` is required diff --git a/rust/src/scannerctl/linter/lints/snapshots/scannerctl__linter__lints__script_xref__tests__malformed_script_xrefs.snap b/rust/src/scannerctl/linter/lints/snapshots/scannerctl__linter__lints__script_xref__tests__malformed_script_xrefs.snap new file mode 100644 index 0000000000..506a903340 --- /dev/null +++ b/rust/src/scannerctl/linter/lints/snapshots/scannerctl__linter__lints__script_xref__tests__malformed_script_xrefs.snap @@ -0,0 +1,35 @@ +--- +source: src/scannerctl/linter/lints/script_xref.rs +expression: "$crate :: linter :: tests ::\nlint(stringify! (malformed_script_xrefs),\nr#\"\nscript_xref(value: \"https://example.com\");\nscript_xref(name: \"URL\");\nscript_xref();\nscript_xref(name: \"URL\", value: \"https://example.com/a, b\");\nscript_xref(name: \"CVE, GHSA\", csv: \"CVE-2025-0001, CVE-2025-0002\");\n\"#,)" +--- +error: Invalid script_xref call + ┌─ malformed_script_xrefs:2:1 + │ +2 │ script_xref(value: "https://example.com"); + │ ^^^^^^^^^^^ `name` is required + +error: Invalid script_xref call + ┌─ malformed_script_xrefs:3:1 + │ +3 │ script_xref(name: "URL"); + │ ^^^^^^^^^^^ at least one of `value` or `csv` is required + +error: Invalid script_xref call + ┌─ malformed_script_xrefs:4:1 + │ +4 │ script_xref(); + │ ^^^^^^^^^^^ `name` and at least one of `value` or `csv` are required + +error: Invalid script_xref call + ┌─ malformed_script_xrefs:5:33 + │ +5 │ script_xref(name: "URL", value: "https://example.com/a, b"); + │ ^^^^^^^^^^^^^^^^^^^^^^^^^^ xref strings must not contain `, ` + +error: Invalid script_xref call + ┌─ malformed_script_xrefs:6:19 + │ +6 │ script_xref(name: "CVE, GHSA", csv: "CVE-2025-0001, CVE-2025-0002"); + │ ^^^^^^^^^^^ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ xref strings must not contain `, ` + │ │ + │ xref strings must not contain `, ` diff --git a/rust/src/scannerctl/linter/lints/snapshots/scannerctl__linter__lints__script_xref__tests__valid_script_xrefs.snap b/rust/src/scannerctl/linter/lints/snapshots/scannerctl__linter__lints__script_xref__tests__valid_script_xrefs.snap new file mode 100644 index 0000000000..5dcca0d4ed --- /dev/null +++ b/rust/src/scannerctl/linter/lints/snapshots/scannerctl__linter__lints__script_xref__tests__valid_script_xrefs.snap @@ -0,0 +1,5 @@ +--- +source: src/scannerctl/linter/lints/script_xref.rs +expression: "$crate :: linter :: tests ::\nlint(stringify! (valid_script_xrefs),\nr#\"\nscript_xref(name: \"URL\", value: \"https://example.com\");\nscript_xref(name: \"CVE\", csv: \"CVE-2025-0001,CVE-2025-0002\");\nscript_xref(name: \"GHSA\", value: \"GHSA-3333-4444-5555\", csv: \"GHSA-1111-2222-3333,GHSA-2222-3333-4444\");\n\"#,)" +--- + diff --git a/rust/src/scannerctl/linter/lints/snapshots/scannerctl__linter__lints__undeclared_variable__tests__assignment_array_index_is_read.snap b/rust/src/scannerctl/linter/lints/snapshots/scannerctl__linter__lints__undeclared_variable__tests__assignment_array_index_is_read.snap new file mode 100644 index 0000000000..528eaf9b45 --- /dev/null +++ b/rust/src/scannerctl/linter/lints/snapshots/scannerctl__linter__lints__undeclared_variable__tests__assignment_array_index_is_read.snap @@ -0,0 +1,9 @@ +--- +source: src/scannerctl/linter/lints/undeclared_variable.rs +expression: "$crate :: linter :: tests ::\nlint(stringify! (assignment_array_index_is_read),\n\"array[index] = 1; display(array);\",)" +--- +error: Variable `index` is not declared + ┌─ assignment_array_index_is_read:1:7 + │ +1 │ array[index] = 1; display(array); + │ ^^^^^ undeclared variable diff --git a/rust/src/scannerctl/linter/lints/snapshots/scannerctl__linter__lints__undeclared_variable__tests__assignment_declares_variable.snap b/rust/src/scannerctl/linter/lints/snapshots/scannerctl__linter__lints__undeclared_variable__tests__assignment_declares_variable.snap new file mode 100644 index 0000000000..abd274c439 --- /dev/null +++ b/rust/src/scannerctl/linter/lints/snapshots/scannerctl__linter__lints__undeclared_variable__tests__assignment_declares_variable.snap @@ -0,0 +1,5 @@ +--- +source: src/scannerctl/linter/lints/undeclared_variable.rs +expression: "$crate :: linter :: tests ::\nlint(stringify! (assignment_declares_variable), \"value = 1; display(value);\",)" +--- + diff --git a/rust/src/scannerctl/linter/lints/snapshots/scannerctl__linter__lints__undeclared_variable__tests__assignment_rhs_is_read.snap b/rust/src/scannerctl/linter/lints/snapshots/scannerctl__linter__lints__undeclared_variable__tests__assignment_rhs_is_read.snap new file mode 100644 index 0000000000..90d7a3edc9 --- /dev/null +++ b/rust/src/scannerctl/linter/lints/snapshots/scannerctl__linter__lints__undeclared_variable__tests__assignment_rhs_is_read.snap @@ -0,0 +1,9 @@ +--- +source: src/scannerctl/linter/lints/undeclared_variable.rs +expression: "$crate :: linter :: tests ::\nlint(stringify! (assignment_rhs_is_read), \"value = missing;\",)" +--- +error: Variable `missing` is not declared + ┌─ assignment_rhs_is_read:1:9 + │ +1 │ value = missing; + │ ^^^^^^^ undeclared variable diff --git a/rust/src/scannerctl/linter/lints/snapshots/scannerctl__linter__lints__undeclared_variable__tests__c_predefined_constants.snap b/rust/src/scannerctl/linter/lints/snapshots/scannerctl__linter__lints__undeclared_variable__tests__c_predefined_constants.snap new file mode 100644 index 0000000000..a8bd94266f --- /dev/null +++ b/rust/src/scannerctl/linter/lints/snapshots/scannerctl__linter__lints__undeclared_variable__tests__c_predefined_constants.snap @@ -0,0 +1,4 @@ +--- +source: src/scannerctl/linter/lints/undeclared_variable.rs +expression: "$crate :: linter :: tests ::\nlint(stringify! (c_predefined_constants),\n\"display(IPPROTO_ICMPV6, MSG_OOB, NOERR, ETIMEDOUT, ECONNRESET, EUNREACH, EUNKNOWN);\",)" +--- diff --git a/rust/src/scannerctl/linter/lints/snapshots/scannerctl__linter__lints__undeclared_variable__tests__foreach_declares_iterator.snap b/rust/src/scannerctl/linter/lints/snapshots/scannerctl__linter__lints__undeclared_variable__tests__foreach_declares_iterator.snap new file mode 100644 index 0000000000..ee182e283e --- /dev/null +++ b/rust/src/scannerctl/linter/lints/snapshots/scannerctl__linter__lints__undeclared_variable__tests__foreach_declares_iterator.snap @@ -0,0 +1,5 @@ +--- +source: src/scannerctl/linter/lints/undeclared_variable.rs +expression: "$crate :: linter :: tests ::\nlint(stringify! (foreach_declares_iterator),\n\"items = [1]; foreach item(items) { display(item); }\",)" +--- + diff --git a/rust/src/scannerctl/linter/lints/snapshots/scannerctl__linter__lints__undeclared_variable__tests__function_assignment_does_not_leak.snap b/rust/src/scannerctl/linter/lints/snapshots/scannerctl__linter__lints__undeclared_variable__tests__function_assignment_does_not_leak.snap new file mode 100644 index 0000000000..d787a6297a --- /dev/null +++ b/rust/src/scannerctl/linter/lints/snapshots/scannerctl__linter__lints__undeclared_variable__tests__function_assignment_does_not_leak.snap @@ -0,0 +1,9 @@ +--- +source: src/scannerctl/linter/lints/undeclared_variable.rs +expression: "$crate :: linter :: tests ::\nlint(stringify! (function_assignment_does_not_leak),\n\"function foo() { scoped = 1; } display(scoped);\",)" +--- +error: Variable `scoped` is not declared + ┌─ function_assignment_does_not_leak:1:40 + │ +1 │ function foo() { scoped = 1; } display(scoped); + │ ^^^^^^ undeclared variable diff --git a/rust/src/scannerctl/linter/lints/snapshots/scannerctl__linter__lints__undeclared_variable__tests__function_global_is_visible_elsewhere.snap b/rust/src/scannerctl/linter/lints/snapshots/scannerctl__linter__lints__undeclared_variable__tests__function_global_is_visible_elsewhere.snap new file mode 100644 index 0000000000..70827a8192 --- /dev/null +++ b/rust/src/scannerctl/linter/lints/snapshots/scannerctl__linter__lints__undeclared_variable__tests__function_global_is_visible_elsewhere.snap @@ -0,0 +1,5 @@ +--- +source: src/scannerctl/linter/lints/undeclared_variable.rs +expression: "$crate :: linter :: tests ::\nlint(stringify! (function_global_is_visible_elsewhere),\n\"function define() { global_var shared; } function use() { display(shared); } display(shared);\",)" +--- + diff --git a/rust/src/scannerctl/linter/lints/snapshots/scannerctl__linter__lints__undeclared_variable__tests__function_local_does_not_leak.snap b/rust/src/scannerctl/linter/lints/snapshots/scannerctl__linter__lints__undeclared_variable__tests__function_local_does_not_leak.snap new file mode 100644 index 0000000000..c7968af02c --- /dev/null +++ b/rust/src/scannerctl/linter/lints/snapshots/scannerctl__linter__lints__undeclared_variable__tests__function_local_does_not_leak.snap @@ -0,0 +1,9 @@ +--- +source: src/scannerctl/linter/lints/undeclared_variable.rs +expression: "$crate :: linter :: tests ::\nlint(stringify! (function_local_does_not_leak),\n\"function foo() { local_var scoped; } display(scoped);\",)" +--- +error: Variable `scoped` is not declared + ┌─ function_local_does_not_leak:1:46 + │ +1 │ function foo() { local_var scoped; } display(scoped); + │ ^^^^^^ undeclared variable diff --git a/rust/src/scannerctl/linter/lints/snapshots/scannerctl__linter__lints__undeclared_variable__tests__function_scope.snap b/rust/src/scannerctl/linter/lints/snapshots/scannerctl__linter__lints__undeclared_variable__tests__function_scope.snap new file mode 100644 index 0000000000..e7765520ec --- /dev/null +++ b/rust/src/scannerctl/linter/lints/snapshots/scannerctl__linter__lints__undeclared_variable__tests__function_scope.snap @@ -0,0 +1,9 @@ +--- +source: src/scannerctl/linter/lints/undeclared_variable.rs +expression: "$crate :: linter :: tests ::\nlint(stringify! (function_scope),\n\"function foo(arg) { local_var value; value = arg; display(value); display(missing); }\",)" +--- +error: Variable `missing` is not declared + ┌─ function_scope:1:75 + │ +1 │ function foo(arg) { local_var value; value = arg; display(value); display(missing); } + │ ^^^^^^^ undeclared variable diff --git a/rust/src/scannerctl/linter/lints/snapshots/scannerctl__linter__lints__undeclared_variable__tests__function_use_before_assignment.snap b/rust/src/scannerctl/linter/lints/snapshots/scannerctl__linter__lints__undeclared_variable__tests__function_use_before_assignment.snap new file mode 100644 index 0000000000..252078235a --- /dev/null +++ b/rust/src/scannerctl/linter/lints/snapshots/scannerctl__linter__lints__undeclared_variable__tests__function_use_before_assignment.snap @@ -0,0 +1,9 @@ +--- +source: src/scannerctl/linter/lints/undeclared_variable.rs +expression: "$crate :: linter :: tests ::\nlint(stringify! (function_use_before_assignment),\n\"function foo() { display(scoped); scoped = 1; }\",)" +--- +error: Variable `scoped` is not declared + ┌─ function_use_before_assignment:1:26 + │ +1 │ function foo() { display(scoped); scoped = 1; } + │ ^^^^^^ undeclared variable diff --git a/rust/src/scannerctl/linter/lints/snapshots/scannerctl__linter__lints__undeclared_variable__tests__predefined_variables.snap b/rust/src/scannerctl/linter/lints/snapshots/scannerctl__linter__lints__undeclared_variable__tests__predefined_variables.snap new file mode 100644 index 0000000000..32e2dc7cd9 --- /dev/null +++ b/rust/src/scannerctl/linter/lints/snapshots/scannerctl__linter__lints__undeclared_variable__tests__predefined_variables.snap @@ -0,0 +1,5 @@ +--- +source: src/scannerctl/linter/lints/undeclared_variable.rs +expression: "$crate :: linter :: tests ::\nlint(stringify! (predefined_variables),\n\"if (description) { display(ACT_UNKNOWN, NASL_ERR_NOERR); }\",)" +--- + diff --git a/rust/src/scannerctl/linter/lints/snapshots/scannerctl__linter__lints__undeclared_variable__tests__top_level_local_is_global.snap b/rust/src/scannerctl/linter/lints/snapshots/scannerctl__linter__lints__undeclared_variable__tests__top_level_local_is_global.snap new file mode 100644 index 0000000000..bf045b834e --- /dev/null +++ b/rust/src/scannerctl/linter/lints/snapshots/scannerctl__linter__lints__undeclared_variable__tests__top_level_local_is_global.snap @@ -0,0 +1,5 @@ +--- +source: src/scannerctl/linter/lints/undeclared_variable.rs +expression: "$crate :: linter :: tests ::\nlint(stringify! (top_level_local_is_global),\n\"local_var shared; function use() { display(shared); }\",)" +--- + diff --git a/rust/src/scannerctl/linter/lints/snapshots/scannerctl__linter__lints__undeclared_variable__tests__undeclared_variable.snap b/rust/src/scannerctl/linter/lints/snapshots/scannerctl__linter__lints__undeclared_variable__tests__undeclared_variable.snap new file mode 100644 index 0000000000..bbb3293c5b --- /dev/null +++ b/rust/src/scannerctl/linter/lints/snapshots/scannerctl__linter__lints__undeclared_variable__tests__undeclared_variable.snap @@ -0,0 +1,9 @@ +--- +source: src/scannerctl/linter/lints/undeclared_variable.rs +expression: "$crate :: linter :: tests ::\nlint(stringify! (undeclared_variable), \"display(missing);\",)" +--- +error: Variable `missing` is not declared + ┌─ undeclared_variable:1:9 + │ +1 │ display(missing); + │ ^^^^^^^ undeclared variable diff --git a/rust/src/scannerctl/linter/lints/snapshots/scannerctl__linter__lints__undeclared_variable__tests__use_before_assignment.snap b/rust/src/scannerctl/linter/lints/snapshots/scannerctl__linter__lints__undeclared_variable__tests__use_before_assignment.snap new file mode 100644 index 0000000000..d68c40d71f --- /dev/null +++ b/rust/src/scannerctl/linter/lints/snapshots/scannerctl__linter__lints__undeclared_variable__tests__use_before_assignment.snap @@ -0,0 +1,9 @@ +--- +source: src/scannerctl/linter/lints/undeclared_variable.rs +expression: "$crate :: linter :: tests ::\nlint(stringify! (use_before_assignment), \"display(value); value = 1;\",)" +--- +error: Variable `value` is not declared + ┌─ use_before_assignment:1:9 + │ +1 │ display(value); value = 1; + │ ^^^^^ undeclared variable diff --git a/rust/src/scannerctl/linter/lints/snapshots/scannerctl__linter__lints__unused_include__tests__include_with_top_level_code_is_not_reported.snap b/rust/src/scannerctl/linter/lints/snapshots/scannerctl__linter__lints__unused_include__tests__include_with_top_level_code_is_not_reported.snap new file mode 100644 index 0000000000..84b175eb73 --- /dev/null +++ b/rust/src/scannerctl/linter/lints/snapshots/scannerctl__linter__lints__unused_include__tests__include_with_top_level_code_is_not_reported.snap @@ -0,0 +1,5 @@ +--- +source: src/scannerctl/linter/lints/unused_include.rs +expression: "$crate :: linter :: tests ::\nlint_files(& [\"root.nasl\"], &\n[(\"root.nasl\", \"include(\\\"constants.inc\\\"); display(VALUE);\"),\n(\"constants.inc\", \"VALUE = 1;\")],)" +--- + diff --git a/rust/src/scannerctl/linter/lints/snapshots/scannerctl__linter__lints__unused_include__tests__unused_function_include.snap b/rust/src/scannerctl/linter/lints/snapshots/scannerctl__linter__lints__unused_include__tests__unused_function_include.snap new file mode 100644 index 0000000000..720adf33ee --- /dev/null +++ b/rust/src/scannerctl/linter/lints/snapshots/scannerctl__linter__lints__unused_include__tests__unused_function_include.snap @@ -0,0 +1,9 @@ +--- +source: src/scannerctl/linter/lints/unused_include.rs +expression: "$crate :: linter :: tests ::\nlint_files(& [\"root.nasl\"], &\n[(\"root.nasl\", \"include(\\\"helper.inc\\\");\"),\n(\"helper.inc\", \"function helper() {}\")],)" +--- +warning: Included file 'helper.inc' is never used + ┌─ root.nasl:1:9 + │ +1 │ include("helper.inc"); + │ ^^^^^^^^^^^^ unused include diff --git a/rust/src/scannerctl/linter/lints/snapshots/scannerctl__linter__lints__unused_include__tests__unused_include_in_shared_parent_is_emitted_once.snap b/rust/src/scannerctl/linter/lints/snapshots/scannerctl__linter__lints__unused_include__tests__unused_include_in_shared_parent_is_emitted_once.snap new file mode 100644 index 0000000000..dbb0dcf4ce --- /dev/null +++ b/rust/src/scannerctl/linter/lints/snapshots/scannerctl__linter__lints__unused_include__tests__unused_include_in_shared_parent_is_emitted_once.snap @@ -0,0 +1,9 @@ +--- +source: src/scannerctl/linter/lints/unused_include.rs +expression: "$crate :: linter :: tests ::\nlint_files(& [\"a.nasl\", \"b.nasl\"], &\n[(\"a.nasl\", \"include(\\\"common.inc\\\"); common();\"),\n(\"b.nasl\", \"include(\\\"common.inc\\\"); common();\"),\n(\"common.inc\", \"include(\\\"unused.inc\\\"); function common() {}\"),\n(\"unused.inc\", \"function unused() {}\")],)" +--- +warning: Included file 'unused.inc' is never used + ┌─ common.inc:1:9 + │ +1 │ include("unused.inc"); function common() {} + │ ^^^^^^^^^^^^ unused include diff --git a/rust/src/scannerctl/linter/lints/snapshots/scannerctl__linter__lints__unused_include__tests__used_function_include.snap b/rust/src/scannerctl/linter/lints/snapshots/scannerctl__linter__lints__unused_include__tests__used_function_include.snap new file mode 100644 index 0000000000..f17273db69 --- /dev/null +++ b/rust/src/scannerctl/linter/lints/snapshots/scannerctl__linter__lints__unused_include__tests__used_function_include.snap @@ -0,0 +1,5 @@ +--- +source: src/scannerctl/linter/lints/unused_include.rs +expression: "$crate :: linter :: tests ::\nlint_files(& [\"root.nasl\"], &\n[(\"root.nasl\", \"include(\\\"helper.inc\\\"); helper();\"),\n(\"helper.inc\", \"function helper() {}\")],)" +--- + diff --git a/rust/src/scannerctl/linter/lints/snapshots/scannerctl__linter__lints__unused_include__tests__used_transitive_function_includes.snap b/rust/src/scannerctl/linter/lints/snapshots/scannerctl__linter__lints__unused_include__tests__used_transitive_function_includes.snap new file mode 100644 index 0000000000..caf4afdda2 --- /dev/null +++ b/rust/src/scannerctl/linter/lints/snapshots/scannerctl__linter__lints__unused_include__tests__used_transitive_function_includes.snap @@ -0,0 +1,5 @@ +--- +source: src/scannerctl/linter/lints/unused_include.rs +expression: "$crate :: linter :: tests ::\nlint_files(& [\"root.nasl\"], &\n[(\"root.nasl\", \"include(\\\"wrapper.inc\\\"); wrapper();\"),\n(\"wrapper.inc\", \"include(\\\"helper.inc\\\"); function wrapper() { helper(); }\"),\n(\"helper.inc\", \"function helper() {}\")],)" +--- + diff --git a/rust/src/scannerctl/linter/lints/undeclared_variable.rs b/rust/src/scannerctl/linter/lints/undeclared_variable.rs new file mode 100644 index 0000000000..c7b2cc2ce9 --- /dev/null +++ b/rust/src/scannerctl/linter/lints/undeclared_variable.rs @@ -0,0 +1,259 @@ +use std::{collections::HashSet, mem}; + +use codespan_reporting::diagnostic::{Diagnostic, Label}; +use scannerlib::nasl::{ + SourceFile, + error::Spanned, + syntax::{ + Ident, Visitor, + grammar::{Assignment, Atom, FnDecl, ForEach, Include, Increment, VarScope, VarScopeDecl}, + walk_ast, walk_block, + }, +}; + +use crate::linter::{ + LintMsg, + ctx::{Cache, LintCtx}, + paths::{IncludePath, ResolvedPath}, +}; + +const RULE: &str = "undeclared_variable"; + +struct Scope { + globals: HashSet, + locals: HashSet, +} + +impl Scope { + fn declare_global(&mut self, ident: &Ident) { + self.globals.insert(ident.to_string()); + } + + fn declare_local(&mut self, ident: &Ident) { + self.locals.insert(ident.to_string()); + } + + fn contains(&self, ident: &Ident) -> bool { + let name = ident.to_string(); + self.locals.contains(&name) || self.globals.contains(&name) + } +} + +#[derive(Clone, Copy)] +enum ScopeKind { + File, + Function, +} + +struct OrderedVariables<'cache> { + cache: &'cache Cache, + scope: Scope, + scope_kind: ScopeKind, + path: ResolvedPath, + file: SourceFile, + loaded: HashSet, + messages: Vec, +} + +impl<'cache> OrderedVariables<'cache> { + fn new( + cache: &'cache Cache, + path: ResolvedPath, + file: SourceFile, + globals: HashSet, + scope_kind: ScopeKind, + ) -> Self { + let loaded = HashSet::from([path.clone()]); + Self { + cache, + scope: Scope { + globals, + locals: HashSet::new(), + }, + scope_kind, + path, + file, + loaded, + messages: vec![], + } + } + + fn declare_explicit(&mut self, declaration: &VarScopeDecl) { + for ident in &declaration.idents { + match (self.scope_kind, &declaration.scope) { + (ScopeKind::File, _) | (ScopeKind::Function, VarScope::Global) => { + self.scope.declare_global(ident); + } + (ScopeKind::Function, VarScope::Local) => self.scope.declare_local(ident), + } + } + } + + fn declare_implicit(&mut self, ident: &Ident) { + match self.scope_kind { + ScopeKind::File => self.scope.declare_global(ident), + ScopeKind::Function if !self.scope.contains(ident) => self.scope.declare_local(ident), + ScopeKind::Function => {} + } + } + + fn declare_iterator(&mut self, ident: &Ident) { + match self.scope_kind { + ScopeKind::File => self.scope.declare_global(ident), + // Welcome to NASL where iterator vars still exist after loops + ScopeKind::Function => self.scope.declare_local(ident), + } + } + + fn check_use(&mut self, ident: &Ident) { + if self.scope.contains(ident) { + return; + } + + let name = ident.to_string(); + let message = format!("Variable `{name}` is not declared"); + let span = ident.span(); + let diagnostic = Diagnostic::error().with_message(&message).with_labels(vec![ + Label::primary((), span).with_message("undeclared variable"), + ]); + self.messages + .push(LintMsg::new(RULE, self.file.clone(), span, diagnostic)); + } + + fn check_function(&mut self, declaration: &FnDecl) { + let mut function = OrderedVariables::new( + self.cache, + self.path.clone(), + self.file.clone(), + self.scope.globals.clone(), + ScopeKind::Function, + ); + for argument in &declaration.args.items { + function.scope.declare_local(argument); + } + walk_block(&mut function, &declaration.block); + + self.scope.globals = function.scope.globals; + self.messages.append(&mut function.messages); + } + + fn check_include(&mut self, include: &Include) { + let include_path = IncludePath::new(include.path.as_str()); + let Some((path, ast, file)) = self + .cache + .included_file(&self.path, &include_path) + .map(|(path, cached)| (path.clone(), cached.ast().clone(), cached.file().clone())) + else { + return; + }; + if !self.loaded.insert(path.clone()) { + return; + } + + let outer_path = mem::replace(&mut self.path, path); + let outer_file = mem::replace(&mut self.file, file); + walk_ast(self, &ast); + self.file = outer_file; + self.path = outer_path; + } +} + +impl<'ast> Visitor<'ast> for OrderedVariables<'_> { + fn visit_var_scope_decl(&mut self, declaration: &'ast VarScopeDecl) { + self.declare_explicit(declaration); + } + + fn visit_fn_decl(&mut self, declaration: &'ast FnDecl) { + self.check_function(declaration); + } + + fn visit_include(&mut self, include: &'ast Include) { + self.check_include(include); + } + + fn visit_atom(&mut self, atom: &'ast Atom) { + if let Atom::Ident(ident) = atom { + self.check_use(ident); + } + } + + fn leave_assignment(&mut self, assignment: &'ast Assignment) { + self.declare_implicit(&assignment.lhs.ident); + } + + fn leave_increment(&mut self, increment: &'ast Increment) { + self.declare_implicit(&increment.expr.ident); + } + + fn visit_for_each_binding(&mut self, for_each: &'ast ForEach) { + self.declare_iterator(&for_each.var); + } + + fn should_walk_fn_body(&self, _declaration: &'ast FnDecl) -> bool { + false + } +} + +pub fn undeclared_variables(ctx: &LintCtx) -> Vec { + let cache = &*ctx.cache; + let globals = cache.predefined_vars().map(str::to_owned).collect(); + let mut variables = OrderedVariables::new( + cache, + ctx.path.clone(), + ctx.file.clone(), + globals, + ScopeKind::File, + ); + walk_ast(&mut variables, ctx.ast); + variables.messages +} + +#[cfg(test)] +mod tests { + use crate::linter_test; + + linter_test!(undeclared_variable, "display(missing);"); + linter_test!(assignment_declares_variable, "value = 1; display(value);"); + linter_test!(use_before_assignment, "display(value); value = 1;"); + linter_test!(assignment_rhs_is_read, "value = missing;"); + linter_test!( + assignment_array_index_is_read, + "array[index] = 1; display(array);" + ); + linter_test!( + function_scope, + "function foo(arg) { local_var value; value = arg; display(value); display(missing); }" + ); + linter_test!( + function_local_does_not_leak, + "function foo() { local_var scoped; } display(scoped);" + ); + linter_test!( + function_assignment_does_not_leak, + "function foo() { scoped = 1; } display(scoped);" + ); + linter_test!( + function_use_before_assignment, + "function foo() { display(scoped); scoped = 1; }" + ); + linter_test!( + function_global_is_visible_elsewhere, + "function define() { global_var shared; } function use() { display(shared); } display(shared);" + ); + linter_test!( + top_level_local_is_global, + "local_var shared; function use() { display(shared); }" + ); + linter_test!( + foreach_declares_iterator, + "items = [1]; foreach item(items) { display(item); }" + ); + linter_test!( + predefined_variables, + "if (description) { display(ACT_UNKNOWN, NASL_ERR_NOERR); }" + ); + linter_test!( + c_predefined_constants, + "display(IPPROTO_ICMPV6, MSG_OOB, NOERR, ETIMEDOUT, ECONNRESET, EUNREACH, EUNKNOWN);" + ); +} diff --git a/rust/src/scannerctl/linter/lints/unused_include.rs b/rust/src/scannerctl/linter/lints/unused_include.rs new file mode 100644 index 0000000000..f020aef575 --- /dev/null +++ b/rust/src/scannerctl/linter/lints/unused_include.rs @@ -0,0 +1,180 @@ +use std::collections::{HashMap, HashSet}; + +use codespan_reporting::diagnostic::{Diagnostic, Label}; +use scannerlib::nasl::syntax::grammar::{Include, Statement}; + +use crate::linter::{ + LintMsg, + ctx::{Cache, LintCtx}, + paths::{IncludePath, ResolvedPath}, +}; + +const RULE: &str = "unused_include"; + +// Some include files may contain top level statements +// that are executed when the file is included. Such files +// should never be reported as unused includes. The terminology +// here is that "library" refers to a file that only provides +// function declarations and executes no top level statement. +// +// This function recursively collects all such library files. +fn collect_function_library( + cache: &Cache, + path: &ResolvedPath, + files: &mut HashSet, + visiting: &mut HashSet, +) -> bool { + if visiting.contains(path) { + return false; + } + if files.contains(path) { + return true; + } + files.insert(path.clone()); + visiting.insert(path.clone()); + + let is_library = cache.file(path).is_some_and(|file| { + file.ast().iter_root_stmts().all(|statement| { + matches!( + statement, + Statement::FnDecl(_) | Statement::Include(_) | Statement::NoOp + ) + }) && file.ast().iter_includes().all(|include| { + let include_path = IncludePath::new(include.path.as_str()); + cache + .included_path(path, &include_path) + .is_some_and(|included_path| { + collect_function_library(cache, included_path, files, visiting) + }) + }) + }); + visiting.remove(path); + is_library +} + +fn function_library(cache: &Cache, path: &ResolvedPath) -> Option> { + let mut files = HashSet::new(); + let mut visiting = HashSet::new(); + collect_function_library(cache, path, &mut files, &mut visiting).then_some(files) +} + +fn function_callers(cache: &Cache) -> HashMap> { + let mut callers = HashMap::>::new(); + for (path, file) in cache.files() { + for call in file.ast().iter_fn_calls() { + callers + .entry(call.fn_name.to_string()) + .or_default() + .insert(path.clone()); + } + } + callers +} + +fn is_used( + cache: &Cache, + callers: &HashMap>, + library_files: &HashSet, +) -> bool { + // An include is used when code outside that include chain calls a function + // it provides. Calls between files in the same include chain do not count. + library_files + .iter() + .filter_map(|path| cache.file(path)) + .flat_map(|file| file.functions()) + .any(|(name, _)| { + callers.get(name).is_some_and(|caller_files| { + caller_files + .iter() + .any(|path| !library_files.contains(path)) + }) + }) +} + +fn message(file: &scannerlib::nasl::SourceFile, include: &Include) -> LintMsg { + let text = format!("Included file '{}' is never used", include.path); + let diagnostic = Diagnostic::warning().with_message(&text).with_labels(vec![ + Label::primary((), include.span).with_message("unused include"), + ]); + LintMsg::new(RULE, file.clone(), include.span, diagnostic) +} + +pub fn unused_includes(ctx: &LintCtx) -> Vec { + let cache = &*ctx.cache; + let mut files = cache.files().collect::>(); + files.sort_by(|(left, _), (right, _)| left.cmp(right)); + let callers = function_callers(cache); + let mut libraries = HashMap::>>::new(); + let mut messages = vec![]; + + for (path, file) in files { + for include in file.ast().iter_includes() { + let include_path = IncludePath::new(include.path.as_str()); + let Some(included_path) = cache.included_path(path, &include_path) else { + continue; + }; + let library_files = libraries + .entry(included_path.clone()) + .or_insert_with(|| function_library(cache, included_path)); + if let Some(library_files) = library_files + && !is_used(cache, &callers, library_files) + { + messages.push(message(file.file(), include)); + } + } + } + messages +} + +#[cfg(test)] +mod tests { + use crate::linter_test_multi; + + linter_test_multi!( + unused_function_include, + roots: ["root.nasl"], + files: { + "root.nasl" => "include(\"helper.inc\");", + "helper.inc" => "function helper() {}", + }, + ); + + linter_test_multi!( + used_function_include, + roots: ["root.nasl"], + files: { + "root.nasl" => "include(\"helper.inc\"); helper();", + "helper.inc" => "function helper() {}", + }, + ); + + linter_test_multi!( + used_transitive_function_includes, + roots: ["root.nasl"], + files: { + "root.nasl" => "include(\"wrapper.inc\"); wrapper();", + "wrapper.inc" => "include(\"helper.inc\"); function wrapper() { helper(); }", + "helper.inc" => "function helper() {}", + }, + ); + + linter_test_multi!( + include_with_top_level_code_is_not_reported, + roots: ["root.nasl"], + files: { + "root.nasl" => "include(\"constants.inc\"); display(VALUE);", + "constants.inc" => "VALUE = 1;", + }, + ); + + linter_test_multi!( + unused_include_in_shared_parent_is_emitted_once, + roots: ["a.nasl", "b.nasl"], + files: { + "a.nasl" => "include(\"common.inc\"); common();", + "b.nasl" => "include(\"common.inc\"); common();", + "common.inc" => "include(\"unused.inc\"); function common() {}", + "unused.inc" => "function unused() {}", + }, + ); +} diff --git a/rust/src/scannerctl/linter/mod.rs b/rust/src/scannerctl/linter/mod.rs index 02b4d8188b..7c6e1dac7a 100644 --- a/rust/src/scannerctl/linter/mod.rs +++ b/rust/src/scannerctl/linter/mod.rs @@ -1,16 +1,22 @@ mod cli; mod ctx; mod lints; +mod paths; #[cfg(test)] pub(crate) mod tests; -use std::path::PathBuf; +use std::{ + collections::{HashMap, HashSet}, + path::{Path, PathBuf}, + sync::Arc, +}; pub use cli::LinterArgs; use cli::get_files_and_loader; use codespan_reporting::diagnostic::{Diagnostic, Label}; use ctx::{Cache, CachedFile, LintCtx}; -use lints::{Lint, LintMsg, all_lints}; +use lints::{Lint, LintMsg, LintMsgKey, all_lints}; +use paths::{IncludePath, ResolvedPath}; use scannerlib::nasl::{ Code, Loader, SourceFile, error::{IntoDiagnostic, emit_errors}, @@ -22,6 +28,13 @@ use scannerlib::nasl::{ use crate::error::{CliError, CliErrorKind}; +type ParsedFile = Result, Vec>; + +struct ResolvedInclude { + path: ResolvedPath, + parsed: ParsedFile, +} + #[derive(Default)] struct Statistics { checked: usize, @@ -39,11 +52,8 @@ struct Linter { lints: Vec>, cache: Cache, -} - -struct LintMsgs { - file: SourceFile, - msgs: Vec, + parsed_includes: HashMap, + lint_msgs: HashSet, } impl Linter { @@ -69,74 +79,155 @@ impl Linter { } } - fn lint_file(&mut self, rel_path: &str) -> Result { - let code = self.load(rel_path)?; - let file = code.file(); - let ast = match self.parse_file(code) { - Ok(ast) => ast, - Err(msgs) => return Ok(LintMsgs { file, msgs }), + fn lint_file(&mut self, rel_path: &str) -> Result, LoadError> { + self.cache.clear_files(); + let root_path = ResolvedPath::new(rel_path); + let root = match self.get_or_parse_root(&root_path)? { + Ok(root) => root, + Err(msgs) => return Ok(msgs), }; + self.cache.insert(&root_path, root.clone()); + let mut loaded = HashSet::from([root_path.clone()]); + let root_dir = root_path.as_path().parent().unwrap_or(Path::new("")); + if let Err(msgs) = + self.load_includes(root.ast(), root.file(), &root_path, root_dir, &mut loaded) + { + return Ok(msgs); + } + let msgs = if self.only_syntax { vec![] } else { - for include in ast.iter_includes() { - let code = match self.load(&include.path) { - Ok(code) => code, - Err(_) => { - // TODO report multiple errors here if multiple files - // cannot be found. - return Ok(LintMsgs { - file, - msgs: vec![make_load_error_msg(include)], - }); - } - }; - match self.parse_file(code) { - Ok(ast) => { - self.cache.insert(&include.path, CachedFile::new(&ast)); - } - Err(_) => { - todo!() - } - } - } - self.cache.insert(rel_path, CachedFile::new(&ast)); - - let ctx = LintCtx::new(&ast, &mut self.cache); + let ctx = LintCtx::new(root.ast(), root.file(), &root_path, &mut self.cache); self.lints.iter().flat_map(|lint| lint.lint(&ctx)).collect() }; - Ok(LintMsgs { file, msgs }) + Ok(msgs) + } + + fn load_includes( + &mut self, + ast: &Ast, + file: &SourceFile, + parent_path: &ResolvedPath, + root_dir: &Path, + loaded: &mut HashSet, + ) -> Result<(), Vec> { + for include in ast.iter_includes() { + let include_path = IncludePath::new(include.path.as_str()); + let resolved = self + .get_or_parse_include(root_dir, &include_path) + .map_err(|_| vec![make_load_error_msg(file.clone(), include)])?; + let included = resolved.parsed?; + + self.cache + .record_include(parent_path, &include_path, &resolved.path); + if loaded.insert(resolved.path.clone()) { + self.cache.insert(&resolved.path, included.clone()); + self.load_includes( + included.ast(), + included.file(), + &resolved.path, + root_dir, + loaded, + )?; + } + } + Ok(()) + } + + fn get_or_parse_root(&self, path: &ResolvedPath) -> Result { + if let Some(parsed) = self.parsed_includes.get(path) { + return Ok(parsed.clone()); + } + self.parse_path(path) + } + + fn get_or_parse_cached(&mut self, path: &ResolvedPath) -> Result { + if let Some(parsed) = self.parsed_includes.get(path) { + return Ok(parsed.clone()); + } + + let parsed = self.parse_path(path)?; + self.parsed_includes.insert(path.clone(), parsed.clone()); + Ok(parsed) + } + + fn get_or_parse_include( + &mut self, + root_dir: &Path, + include_path: &IncludePath, + ) -> Result { + // The C linter runs from the root script's directory, then searches its + // configured include directory. For a feed scan, that directory is the + // feed root. + let root_relative = ResolvedPath::new(root_dir.join(include_path.as_str())); + let feed_relative = ResolvedPath::new(include_path.as_str()); + match self.get_or_parse_cached(&root_relative) { + Ok(parsed) => Ok(ResolvedInclude { + path: root_relative, + parsed, + }), + Err(error) if root_relative == feed_relative => Err(error), + Err(_) => self + .get_or_parse_cached(&feed_relative) + .map(|parsed| ResolvedInclude { + path: feed_relative, + parsed, + }), + } + } + + fn parse_path(&self, path: &ResolvedPath) -> Result { + let code = self.load(path)?; + let file = code.file(); + Ok(self + .parse_file(code) + .map(|ast| Arc::new(CachedFile::new(file, &ast)))) } fn parse_file(&self, code: Code) -> Result> { + let file = code.file(); let parsed = code.parse(); let result = parsed.result(); result.map_err(|e| { e.into_iter() - .map(ParseError::into_diagnostic) - .map(|diagnostic| diagnostic.into()) + .map(|error| { + let span = error.span; + let diagnostic = ParseError::into_diagnostic(error); + LintMsg::new("syntax", file.clone(), span, diagnostic) + }) .collect() }) } - fn load(&mut self, rel_path: &str) -> Result { - Code::load(&self.loader, rel_path) + fn load(&self, path: &ResolvedPath) -> Result { + Code::load(&self.loader, path.as_path()) } - fn handle_msgs(&mut self, msgs: LintMsgs) { - self.stats.errors += msgs.msgs.len(); + fn handle_msgs(&mut self, msgs: Vec) { + let msgs = self.deduplicate(msgs); + self.stats.errors += msgs.len(); if !self.quiet { - emit_errors(&msgs.file, msgs.msgs.into_iter()); + for msg in msgs { + let file = msg.file().clone(); + emit_errors(&file, std::iter::once(msg)); + } } } + + fn deduplicate(&mut self, msgs: Vec) -> Vec { + msgs.into_iter() + .filter(|msg| self.lint_msgs.insert(msg.message_key())) + .collect() + } } -fn make_load_error_msg(include: &Include) -> LintMsg { - let msg = format!("Could not find file '{:?}'", include.path); - Diagnostic::error() +fn make_load_error_msg(file: SourceFile, include: &Include) -> LintMsg { + let msg = format!("Could not find file {:?}", include.path); + let diagnostic = Diagnostic::error() .with_message(&msg) - .with_labels(vec![Label::primary((), include.span).with_message(&msg)]) - .into() + .with_labels(vec![Label::primary((), include.span).with_message(&msg)]); + LintMsg::new("include_not_found", file, include.span, diagnostic) } pub(crate) async fn run( @@ -154,6 +245,8 @@ pub(crate) async fn run( lints, stats: Statistics::default(), cache: Cache::default(), + parsed_includes: HashMap::new(), + lint_msgs: HashSet::new(), loader, }; diff --git a/rust/src/scannerctl/linter/paths.rs b/rust/src/scannerctl/linter/paths.rs new file mode 100644 index 0000000000..7e46017290 --- /dev/null +++ b/rust/src/scannerctl/linter/paths.rs @@ -0,0 +1,38 @@ +use std::{ + fmt, + path::{Path, PathBuf}, +}; + +/// Path text exactly as written in an `include(...)` statement. +#[derive(Clone, Debug, Eq, Hash, PartialEq)] +pub(crate) struct IncludePath(String); + +impl IncludePath { + pub(crate) fn new(path: impl Into) -> Self { + Self(path.into()) + } + + pub(crate) fn as_str(&self) -> &str { + &self.0 + } +} + +/// Loader path and cache identity of a file after include-path resolution. +#[derive(Clone, Debug, Eq, Hash, Ord, PartialEq, PartialOrd)] +pub(crate) struct ResolvedPath(PathBuf); + +impl ResolvedPath { + pub(crate) fn new(path: impl Into) -> Self { + Self(path.into()) + } + + pub(crate) fn as_path(&self) -> &Path { + &self.0 + } +} + +impl fmt::Display for ResolvedPath { + fn fmt(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { + self.0.display().fmt(formatter) + } +} diff --git a/rust/src/scannerctl/linter/tests/mod.rs b/rust/src/scannerctl/linter/tests/mod.rs index 3d2c85910b..4ed2a750e9 100644 --- a/rust/src/scannerctl/linter/tests/mod.rs +++ b/rust/src/scannerctl/linter/tests/mod.rs @@ -1,27 +1,93 @@ -use scannerlib::nasl::{Code, error::emit_errors_str}; - -use crate::linter::{ - ctx::{Cache, CachedFile, LintCtx}, - lints::all_lints, +use std::{ + collections::{HashMap, HashSet}, + fs, }; +use scannerlib::nasl::{Loader, error::emit_errors_str}; + +use crate::linter::{Linter, Statistics, ctx::Cache, lints::all_lints, paths::ResolvedPath}; + +fn make_linter(loader: Loader) -> Linter { + Linter { + verbose: false, + quiet: false, + only_syntax: false, + loader, + stats: Statistics::default(), + lints: all_lints(), + cache: Cache::default(), + parsed_includes: HashMap::new(), + lint_msgs: HashSet::new(), + } +} + pub fn lint(file_name: &str, code: &str) -> String { - let parsed = Code::from_string_filename(code, file_name).parse(); - let file = parsed.file().clone(); - let ast = match parsed.result() { - Ok(ast) => ast, - Err(errs) => { - return emit_errors_str(&file, errs.into_iter()); - } - }; - let mut cache = Cache::default(); - cache.insert(file_name, CachedFile::new(&ast)); - let ctx = LintCtx::new(&ast, &mut cache); - let msgs: Vec<_> = all_lints() + lint_files(&[file_name], &[(file_name, code)]) +} + +pub fn lint_files(roots: &[&str], files: &[(&str, &str)]) -> String { + lint_files_with_options(roots, files, false) +} + +pub fn lint_files_with_options( + roots: &[&str], + files: &[(&str, &str)], + only_syntax: bool, +) -> String { + let loader = files .iter() - .flat_map(|lint| lint.lint(&ctx)) - .collect(); - emit_errors_str(&file, msgs.into_iter()) + .fold(Loader::test(), |loader, (name, code)| { + loader.with_file(name, (*code).into()) + }) + .build(); + let mut linter = make_linter(loader); + linter.only_syntax = only_syntax; + + roots + .iter() + .map(|root| { + let result = linter.lint_file(root).unwrap(); + let result = linter.deduplicate(result); + result + .into_iter() + .map(|msg| { + let file = msg.file().clone(); + emit_errors_str(&file, std::iter::once(msg)) + }) + .collect::() + }) + .collect() +} + +#[test] +fn parsed_include_is_reused_between_roots() { + let directory = tempfile::tempdir().unwrap(); + fs::write( + directory.path().join("first.nasl"), + "include(\"functions.inc\"); helper();", + ) + .unwrap(); + fs::write( + directory.path().join("second.nasl"), + "include(\"functions.inc\"); helper();", + ) + .unwrap(); + let include = directory.path().join("functions.inc"); + fs::write(&include, "function helper() {}").unwrap(); + + let mut linter = make_linter(Loader::from_feed_path(directory.path())); + assert!(linter.lint_file("first.nasl").unwrap().is_empty()); + assert_eq!( + linter + .parsed_includes + .keys() + .map(ResolvedPath::as_path) + .collect::>(), + vec![std::path::Path::new("functions.inc")] + ); + + fs::write(include, "function helper(").unwrap(); + assert!(linter.lint_file("second.nasl").unwrap().is_empty()); } #[macro_export] @@ -33,3 +99,191 @@ macro_rules! linter_test { } }; } + +#[macro_export] +macro_rules! linter_test_multi { + ( + $name:ident, + only_syntax: $only_syntax:literal, + roots: [$($root:literal),+ $(,)?], + files: {$($file:literal => $code:literal),+ $(,)?} $(,)? + ) => { + #[test] + fn $name() { + insta::assert_snapshot!($crate::linter::tests::lint_files_with_options( + &[$($root),+], + &[$(($file, $code)),+], + $only_syntax, + )); + } + }; + ( + $name:ident, + roots: [$($root:literal),+ $(,)?], + files: {$($file:literal => $code:literal),+ $(,)?} $(,)? + ) => { + #[test] + fn $name() { + insta::assert_snapshot!($crate::linter::tests::lint_files( + &[$($root),+], + &[$(($file, $code)),+], + )); + } + }; +} + +linter_test_multi!( + syntax_checks_included_files, + only_syntax: true, + roots: ["root.nasl"], + files: { + "root.nasl" => "include(\"broken.inc\");", + "broken.inc" => "function broken(", + }, +); + +linter_test_multi!( + included_parse_error_uses_included_source, + roots: ["root.nasl"], + files: { + "root.nasl" => "include(\"broken.inc\");", + "broken.inc" => "function foo(", + }, +); + +linter_test_multi!( + function_definitions_do_not_leak_between_files, + roots: ["first.nasl", "second.nasl"], + files: { + "first.nasl" => "function leaked() {}", + "second.nasl" => "leaked();", + }, +); + +linter_test_multi!( + resolves_direct_include, + roots: ["root.nasl"], + files: { + "root.nasl" => "include(\"functions.inc\"); foo();", + "functions.inc" => "function foo() {}", + }, +); + +linter_test_multi!( + resolves_transitive_include, + roots: ["root.nasl"], + files: { + "root.nasl" => "include(\"first.inc\"); foo();", + "first.inc" => "include(\"second.inc\");", + "second.inc" => "function foo() {}", + }, +); + +linter_test_multi!( + resolves_in_root_directory_before_feed_root, + roots: ["nested/root.nasl"], + files: { + "nested/root.nasl" => "include(\"helper.inc\"); include(\"shared.inc\"); local_helper(); shared();", + "nested/helper.inc" => "function local_helper() {}", + "helper.inc" => "function feed_helper() {}", + "shared.inc" => "function shared() {}", + }, +); + +linter_test_multi!( + same_include_name_resolves_independently_per_root, + roots: ["a/root.nasl", "b/root.nasl"], + files: { + "a/root.nasl" => "include(\"helper.inc\"); from_a();", + "a/helper.inc" => "function from_a() {}", + "b/root.nasl" => "include(\"helper.inc\"); from_b();", + "b/helper.inc" => "function from_b() {}", + }, +); + +linter_test_multi!( + handles_include_cycle, + roots: ["root.nasl"], + files: { + "root.nasl" => "include(\"first.inc\");", + "first.inc" => "include(\"root.nasl\");", + }, +); + +linter_test_multi!( + shared_include_parse_error_is_emitted_once, + roots: ["a.nasl", "b.nasl"], + files: { + "a.nasl" => "include(\"common.inc\");", + "b.nasl" => "include(\"common.inc\");", + "common.inc" => "function broken(", + }, +); + +linter_test_multi!( + duplicate_function_declaration_across_includes, + roots: ["a.nasl", "b.nasl"], + files: { + "a.nasl" => "include(\"first.inc\"); include(\"second.inc\"); foo();", + "b.nasl" => "include(\"second.inc\"); include(\"first.inc\"); foo();", + "first.inc" => "function foo() {}", + "second.inc" => "function foo() {}", + }, +); + +linter_test_multi!( + builtin_function_redefinition_in_shared_include_is_emitted_once, + roots: ["a.nasl", "b.nasl"], + files: { + "a.nasl" => "include(\"common.inc\"); display();", + "b.nasl" => "include(\"common.inc\"); display();", + "common.inc" => "function display() {}", + }, +); + +linter_test_multi!( + variable_declared_in_include, + roots: ["root.nasl"], + files: { + "root.nasl" => "include(\"globals.inc\"); display(shared);", + "globals.inc" => "shared = 1;", + }, +); + +linter_test_multi!( + variable_used_before_declaring_include, + roots: ["root.nasl"], + files: { + "root.nasl" => "display(shared); include(\"globals.inc\");", + "globals.inc" => "shared = 1;", + }, +); + +linter_test_multi!( + undeclared_variable_in_shared_include_is_emitted_once, + roots: ["a.nasl", "b.nasl"], + files: { + "a.nasl" => "include(\"common.inc\");", + "b.nasl" => "include(\"common.inc\");", + "common.inc" => "display(missing);", + }, +); + +linter_test_multi!( + function_call_lints_in_include, + roots: ["root.nasl"], + files: { + "root.nasl" => "include(\"calls.inc\");", + "calls.inc" => "missing(value: 1, value: 2);", + }, +); + +linter_test_multi!( + function_call_lints_in_shared_include_are_emitted_once, + roots: ["a.nasl", "b.nasl"], + files: { + "a.nasl" => "include(\"calls.inc\");", + "b.nasl" => "include(\"calls.inc\");", + "calls.inc" => "missing(value: 1, value: 2);", + }, +); diff --git a/rust/src/scannerctl/linter/tests/snapshots/scannerctl__linter__tests__builtin_function_redefinition_in_shared_include_is_emitted_once.snap b/rust/src/scannerctl/linter/tests/snapshots/scannerctl__linter__tests__builtin_function_redefinition_in_shared_include_is_emitted_once.snap new file mode 100644 index 0000000000..fc830a49b3 --- /dev/null +++ b/rust/src/scannerctl/linter/tests/snapshots/scannerctl__linter__tests__builtin_function_redefinition_in_shared_include_is_emitted_once.snap @@ -0,0 +1,10 @@ +--- +source: src/scannerctl/linter/tests/mod.rs +assertion_line: 208 +expression: "$crate :: linter :: tests ::\nlint_files(& [\"a.nasl\", \"b.nasl\"], &\n[(\"a.nasl\", \"include(\\\"common.inc\\\"); display();\"),\n(\"b.nasl\", \"include(\\\"common.inc\\\"); display();\"),\n(\"common.inc\", \"function display() {}\")],)" +--- +error: Cannot redefine built-in function: display + ┌─ common.inc:1:10 + │ +1 │ function display() {} + │ ^^^^^^^ built-in function redefined here diff --git a/rust/src/scannerctl/linter/tests/snapshots/scannerctl__linter__tests__duplicate_function_declaration_across_includes.snap b/rust/src/scannerctl/linter/tests/snapshots/scannerctl__linter__tests__duplicate_function_declaration_across_includes.snap new file mode 100644 index 0000000000..3e2022ffbe --- /dev/null +++ b/rust/src/scannerctl/linter/tests/snapshots/scannerctl__linter__tests__duplicate_function_declaration_across_includes.snap @@ -0,0 +1,12 @@ +--- +source: src/scannerctl/linter/tests/mod.rs +assertion_line: 128 +expression: "$crate :: linter :: tests ::\nlint_files(& [\"a.nasl\", \"b.nasl\"], &\n[(\"a.nasl\", \"include(\\\"first.inc\\\"); include(\\\"second.inc\\\");\"),\n(\"b.nasl\", \"include(\\\"second.inc\\\"); include(\\\"first.inc\\\");\"),\n(\"first.inc\", \"function foo() {}\"), (\"second.inc\", \"function foo() {}\")],)" +--- +error: Function declared multiple times: foo + ┌─ second.inc:1:10 + │ +1 │ function foo() {} + │ ^^^ redeclared here + │ + = also declared in first.inc diff --git a/rust/src/scannerctl/linter/tests/snapshots/scannerctl__linter__tests__function_call_lints_in_include.snap b/rust/src/scannerctl/linter/tests/snapshots/scannerctl__linter__tests__function_call_lints_in_include.snap new file mode 100644 index 0000000000..77f7c5b5cf --- /dev/null +++ b/rust/src/scannerctl/linter/tests/snapshots/scannerctl__linter__tests__function_call_lints_in_include.snap @@ -0,0 +1,17 @@ +--- +source: src/scannerctl/linter/tests/mod.rs +expression: "$crate :: linter :: tests ::\nlint_files(& [\"root.nasl\"], &\n[(\"root.nasl\", \"include(\\\"calls.inc\\\");\"),\n(\"calls.inc\", \"missing(value: 1, value: 2);\")],)" +--- +warning: Function argument passed multiple times: value + ┌─ calls.inc:1:9 + │ +1 │ missing(value: 1, value: 2); + │ ^^^^^ ^^^^^ Also here + │ │ + │ Function argument passed multiple times: value + +error: Undefined function 'missing' + ┌─ calls.inc:1:1 + │ +1 │ missing(value: 1, value: 2); + │ ^^^^^^^ undefined function diff --git a/rust/src/scannerctl/linter/tests/snapshots/scannerctl__linter__tests__function_call_lints_in_shared_include_are_emitted_once.snap b/rust/src/scannerctl/linter/tests/snapshots/scannerctl__linter__tests__function_call_lints_in_shared_include_are_emitted_once.snap new file mode 100644 index 0000000000..e7dade93a6 --- /dev/null +++ b/rust/src/scannerctl/linter/tests/snapshots/scannerctl__linter__tests__function_call_lints_in_shared_include_are_emitted_once.snap @@ -0,0 +1,17 @@ +--- +source: src/scannerctl/linter/tests/mod.rs +expression: "$crate :: linter :: tests ::\nlint_files(& [\"a.nasl\", \"b.nasl\"], &\n[(\"a.nasl\", \"include(\\\"calls.inc\\\");\"), (\"b.nasl\", \"include(\\\"calls.inc\\\");\"),\n(\"calls.inc\", \"missing(value: 1, value: 2);\")],)" +--- +warning: Function argument passed multiple times: value + ┌─ calls.inc:1:9 + │ +1 │ missing(value: 1, value: 2); + │ ^^^^^ ^^^^^ Also here + │ │ + │ Function argument passed multiple times: value + +error: Undefined function 'missing' + ┌─ calls.inc:1:1 + │ +1 │ missing(value: 1, value: 2); + │ ^^^^^^^ undefined function diff --git a/rust/src/scannerctl/linter/tests/snapshots/scannerctl__linter__tests__function_definitions_do_not_leak_between_files.snap b/rust/src/scannerctl/linter/tests/snapshots/scannerctl__linter__tests__function_definitions_do_not_leak_between_files.snap new file mode 100644 index 0000000000..362776bf3f --- /dev/null +++ b/rust/src/scannerctl/linter/tests/snapshots/scannerctl__linter__tests__function_definitions_do_not_leak_between_files.snap @@ -0,0 +1,9 @@ +--- +source: src/scannerctl/linter/tests/mod.rs +expression: "$crate :: linter :: tests ::\nlint_files(& [\"first.nasl\", \"second.nasl\"], &\n[(\"first.nasl\", \"function leaked() {}\"), (\"second.nasl\", \"leaked();\")],)" +--- +error: Undefined function 'leaked' + ┌─ second.nasl:1:1 + │ +1 │ leaked(); + │ ^^^^^^ undefined function diff --git a/rust/src/scannerctl/linter/tests/snapshots/scannerctl__linter__tests__handles_include_cycle.snap b/rust/src/scannerctl/linter/tests/snapshots/scannerctl__linter__tests__handles_include_cycle.snap new file mode 100644 index 0000000000..23a27bdb93 --- /dev/null +++ b/rust/src/scannerctl/linter/tests/snapshots/scannerctl__linter__tests__handles_include_cycle.snap @@ -0,0 +1,4 @@ +--- +source: src/scannerctl/linter/tests/mod.rs +expression: "$crate :: linter :: tests ::\nlint_files(& [\"root.nasl\"], &\n[(\"root.nasl\", \"include(\\\"first.inc\\\");\"),\n(\"first.inc\", \"include(\\\"root.nasl\\\");\")],)" +--- diff --git a/rust/src/scannerctl/linter/tests/snapshots/scannerctl__linter__tests__included_parse_error_uses_included_source.snap b/rust/src/scannerctl/linter/tests/snapshots/scannerctl__linter__tests__included_parse_error_uses_included_source.snap new file mode 100644 index 0000000000..efa559a52b --- /dev/null +++ b/rust/src/scannerctl/linter/tests/snapshots/scannerctl__linter__tests__included_parse_error_uses_included_source.snap @@ -0,0 +1,9 @@ +--- +source: src/scannerctl/linter/tests/mod.rs +expression: "$crate :: linter :: tests ::\nlint_files(& [\"root.nasl\"], &\n[(\"root.nasl\", \"include(\\\"broken.inc\\\");\"), (\"broken.inc\", \"function foo(\")],)" +--- +error: Expected ')' + ┌─ broken.inc:1:1 + │ +1 │ function foo( + │ ^^^^^^^^^^^^^ Expected ')' diff --git a/rust/src/scannerctl/linter/tests/snapshots/scannerctl__linter__tests__resolves_direct_include.snap b/rust/src/scannerctl/linter/tests/snapshots/scannerctl__linter__tests__resolves_direct_include.snap new file mode 100644 index 0000000000..16fbe3514f --- /dev/null +++ b/rust/src/scannerctl/linter/tests/snapshots/scannerctl__linter__tests__resolves_direct_include.snap @@ -0,0 +1,4 @@ +--- +source: src/scannerctl/linter/tests/mod.rs +expression: "$crate :: linter :: tests ::\nlint_files(& [\"root.nasl\"], &\n[(\"root.nasl\", \"include(\\\"functions.inc\\\"); foo();\"),\n(\"functions.inc\", \"function foo() {}\")],)" +--- diff --git a/rust/src/scannerctl/linter/tests/snapshots/scannerctl__linter__tests__resolves_in_root_directory_before_feed_root.snap b/rust/src/scannerctl/linter/tests/snapshots/scannerctl__linter__tests__resolves_in_root_directory_before_feed_root.snap new file mode 100644 index 0000000000..3caccdce09 --- /dev/null +++ b/rust/src/scannerctl/linter/tests/snapshots/scannerctl__linter__tests__resolves_in_root_directory_before_feed_root.snap @@ -0,0 +1,5 @@ +--- +source: src/scannerctl/linter/tests/mod.rs +assertion_line: 178 +expression: "$crate :: linter :: tests ::\nlint_files(& [\"nested/root.nasl\"], &\n[(\"nested/root.nasl\",\n\"include(\\\"helper.inc\\\"); include(\\\"shared.inc\\\"); local_helper(); shared();\"),\n(\"nested/helper.inc\", \"function local_helper() {}\"),\n(\"helper.inc\", \"function feed_helper() {}\"),\n(\"shared.inc\", \"function shared() {}\")],)" +--- diff --git a/rust/src/scannerctl/linter/tests/snapshots/scannerctl__linter__tests__resolves_transitive_include.snap b/rust/src/scannerctl/linter/tests/snapshots/scannerctl__linter__tests__resolves_transitive_include.snap new file mode 100644 index 0000000000..fdb55b68ae --- /dev/null +++ b/rust/src/scannerctl/linter/tests/snapshots/scannerctl__linter__tests__resolves_transitive_include.snap @@ -0,0 +1,4 @@ +--- +source: src/scannerctl/linter/tests/mod.rs +expression: "$crate :: linter :: tests ::\nlint_files(& [\"root.nasl\"], &\n[(\"root.nasl\", \"include(\\\"first.inc\\\"); foo();\"),\n(\"first.inc\", \"include(\\\"second.inc\\\");\"),\n(\"second.inc\", \"function foo() {}\")],)" +--- diff --git a/rust/src/scannerctl/linter/tests/snapshots/scannerctl__linter__tests__same_include_name_resolves_independently_per_root.snap b/rust/src/scannerctl/linter/tests/snapshots/scannerctl__linter__tests__same_include_name_resolves_independently_per_root.snap new file mode 100644 index 0000000000..75f93e205b --- /dev/null +++ b/rust/src/scannerctl/linter/tests/snapshots/scannerctl__linter__tests__same_include_name_resolves_independently_per_root.snap @@ -0,0 +1,5 @@ +--- +source: src/scannerctl/linter/tests/mod.rs +assertion_line: 189 +expression: "$crate :: linter :: tests ::\nlint_files(& [\"a/root.nasl\", \"b/root.nasl\"], &\n[(\"a/root.nasl\", \"include(\\\"helper.inc\\\"); from_a();\"),\n(\"a/helper.inc\", \"function from_a() {}\"),\n(\"b/root.nasl\", \"include(\\\"helper.inc\\\"); from_b();\"),\n(\"b/helper.inc\", \"function from_b() {}\")],)" +--- diff --git a/rust/src/scannerctl/linter/tests/snapshots/scannerctl__linter__tests__shared_include_parse_error_is_emitted_once.snap b/rust/src/scannerctl/linter/tests/snapshots/scannerctl__linter__tests__shared_include_parse_error_is_emitted_once.snap new file mode 100644 index 0000000000..30c578d5bc --- /dev/null +++ b/rust/src/scannerctl/linter/tests/snapshots/scannerctl__linter__tests__shared_include_parse_error_is_emitted_once.snap @@ -0,0 +1,9 @@ +--- +source: src/scannerctl/linter/tests/mod.rs +expression: "$crate :: linter :: tests ::\nlint_files(& [\"a.nasl\", \"b.nasl\"], &\n[(\"a.nasl\", \"include(\\\"common.inc\\\");\"),\n(\"b.nasl\", \"include(\\\"common.inc\\\");\"),\n(\"common.inc\", \"function broken(\")],)" +--- +error: Expected ')' + ┌─ common.inc:1:1 + │ +1 │ function broken( + │ ^^^^^^^^^^^^^^^^ Expected ')' diff --git a/rust/src/scannerctl/linter/tests/snapshots/scannerctl__linter__tests__syntax_checks_included_files.snap b/rust/src/scannerctl/linter/tests/snapshots/scannerctl__linter__tests__syntax_checks_included_files.snap new file mode 100644 index 0000000000..3b5e9f9e7b --- /dev/null +++ b/rust/src/scannerctl/linter/tests/snapshots/scannerctl__linter__tests__syntax_checks_included_files.snap @@ -0,0 +1,9 @@ +--- +source: src/scannerctl/linter/tests/mod.rs +expression: "$crate :: linter :: tests ::\nlint_files_with_options(& [\"root.nasl\"], &\n[(\"root.nasl\", \"include(\\\"broken.inc\\\");\"),\n(\"broken.inc\", \"function broken(\")], true,)" +--- +error: Expected ')' + ┌─ broken.inc:1:1 + │ +1 │ function broken( + │ ^^^^^^^^^^^^^^^^ Expected ')' diff --git a/rust/src/scannerctl/linter/tests/snapshots/scannerctl__linter__tests__undeclared_variable_in_shared_include_is_emitted_once.snap b/rust/src/scannerctl/linter/tests/snapshots/scannerctl__linter__tests__undeclared_variable_in_shared_include_is_emitted_once.snap new file mode 100644 index 0000000000..23ba6cceb5 --- /dev/null +++ b/rust/src/scannerctl/linter/tests/snapshots/scannerctl__linter__tests__undeclared_variable_in_shared_include_is_emitted_once.snap @@ -0,0 +1,9 @@ +--- +source: src/scannerctl/linter/tests/mod.rs +expression: "$crate :: linter :: tests ::\nlint_files(& [\"a.nasl\", \"b.nasl\"], &\n[(\"a.nasl\", \"include(\\\"common.inc\\\");\"),\n(\"b.nasl\", \"include(\\\"common.inc\\\");\"), (\"common.inc\", \"display(missing);\")],)" +--- +error: Variable `missing` is not declared + ┌─ common.inc:1:9 + │ +1 │ display(missing); + │ ^^^^^^^ undeclared variable diff --git a/rust/src/scannerctl/linter/tests/snapshots/scannerctl__linter__tests__variable_declared_in_include.snap b/rust/src/scannerctl/linter/tests/snapshots/scannerctl__linter__tests__variable_declared_in_include.snap new file mode 100644 index 0000000000..a58898a65f --- /dev/null +++ b/rust/src/scannerctl/linter/tests/snapshots/scannerctl__linter__tests__variable_declared_in_include.snap @@ -0,0 +1,5 @@ +--- +source: src/scannerctl/linter/tests/mod.rs +expression: "$crate :: linter :: tests ::\nlint_files(& [\"root.nasl\"], &\n[(\"root.nasl\", \"include(\\\"globals.inc\\\"); display(shared);\"),\n(\"globals.inc\", \"shared = 1;\")],)" +--- + diff --git a/rust/src/scannerctl/linter/tests/snapshots/scannerctl__linter__tests__variable_used_before_declaring_include.snap b/rust/src/scannerctl/linter/tests/snapshots/scannerctl__linter__tests__variable_used_before_declaring_include.snap new file mode 100644 index 0000000000..5df977bc42 --- /dev/null +++ b/rust/src/scannerctl/linter/tests/snapshots/scannerctl__linter__tests__variable_used_before_declaring_include.snap @@ -0,0 +1,9 @@ +--- +source: src/scannerctl/linter/tests/mod.rs +expression: "$crate :: linter :: tests ::\nlint_files(& [\"root.nasl\"], &\n[(\"root.nasl\", \"display(shared); include(\\\"globals.inc\\\");\"),\n(\"globals.inc\", \"shared = 1;\")],)" +--- +error: Variable `shared` is not declared + ┌─ root.nasl:1:9 + │ +1 │ display(shared); include("globals.inc"); + │ ^^^^^^ undeclared variable