diff --git a/cli/src/commands/compile.rs b/cli/src/commands/compile.rs index 90339bb82..c621634f7 100644 --- a/cli/src/commands/compile.rs +++ b/cli/src/commands/compile.rs @@ -3,6 +3,8 @@ use std::path::PathBuf; use anyhow::Context; use clap::{Arg, ArgAction, ArgMatches, Command, arg, value_parser}; +use yansi::Color::Cyan; +use yansi::Paint; use crate::commands::{ compilation_args, compile_rules, path_with_namespace_parser, @@ -32,11 +34,27 @@ pub fn exec_compile(args: &ArgMatches, config: &Config) -> anyhow::Result<()> { .unwrap(); let output_path = args.get_one::("output").unwrap(); - let rules = compile_rules(rules_path, args, config)?; + let (rules, ignored_rules) = compile_rules(rules_path, args, config)?; let output_file = File::create(output_path).with_context(|| { format!("can not write `{}`", output_path.display()) })?; - Ok(rules.serialize_into(&output_file)?) + rules.serialize_into(&output_file)?; + + // With `--ignore-invalid-rules` the valid rules are still written, but exit + // with an error listing the ignored rules and reasons. + if !ignored_rules.is_empty() { + eprintln!( + "{} {}", + "note:".paint(Cyan).bold(), + Paint::bold(&"the following rules were ignored:") + ); + + for (rule_name, reason) in &ignored_rules { + eprintln!(" - {}: {}", rule_name.paint(Cyan).bold(), reason); + } + } + + Ok(()) } diff --git a/cli/src/commands/fix.rs b/cli/src/commands/fix.rs index 562d7a810..407010918 100644 --- a/cli/src/commands/fix.rs +++ b/cli/src/commands/fix.rs @@ -182,7 +182,7 @@ pub fn exec_fix_warnings( .get_many::<(Option, PathBuf)>("[NAMESPACE:]RULES_PATH") .unwrap(); - let rules = compile_rules(rules_path, args, config)?; + let (rules, _) = compile_rules(rules_path, args, config)?; let mut patches_per_origin = HashMap::new(); let mut num_warnings = 0; diff --git a/cli/src/commands/mod.rs b/cli/src/commands/mod.rs index 769f441cb..433ac61bf 100644 --- a/cli/src/commands/mod.rs +++ b/cli/src/commands/mod.rs @@ -40,7 +40,7 @@ use crate::walk::Draw; use crate::walk::Walker; use crate::{APP_HELP_TEMPLATE, commands, help}; -use yara_x::{Compiler, Rules, SourceCode}; +use yara_x::{Compiler, IgnoredRuleReason, Rules, SourceCode}; pub fn command(name: &'static str) -> Command { Command::new(name).help_template( @@ -216,7 +216,7 @@ pub fn create_compiler<'a>( Ok(compiler) } -pub fn compilation_args() -> [Arg; 6] { +pub fn compilation_args() -> [Arg; 7] { [ arg!(-d --"define") .help("Define external variable") @@ -232,6 +232,8 @@ pub fn compilation_args() -> [Arg; 6] { .require_equals(true) .value_delimiter(',') .action(ArgAction::Append), + arg!(--"ignore-invalid-rules") + .help("Ignore rules that fail to compile and continue with the valid ones"), arg!(-I --"ignore-module" ) .help("Ignore rules that use the specified module") .long_help(help::IGNORE_MODULE_LONG_HELP) @@ -252,11 +254,12 @@ pub fn compile_rules<'a, P>( paths: P, args: &ArgMatches, config: &Config, -) -> Result +) -> Result<(Rules, Vec<(String, String)>), anyhow::Error> where P: Iterator, PathBuf)>, { let external_vars = get_external_vars(args); + let ignore_invalid_rules = args.get_flag("ignore-invalid-rules"); let mut compiler = create_compiler(external_vars, args, config)?; let mut pb = if stdout().is_tty() { @@ -336,13 +339,37 @@ where eprintln!("{error}"); } - if !compiler.errors().is_empty() { + let errors_found = !compiler.errors().is_empty(); + + // Without `--ignore-invalid-rules` any compilation error is fatal. With the + // flag, the rules that compiled correctly are kept (the compiler already + // discards only the individual rules that failed) and compilation + // continues. The errors are still reported above. + if errors_found && !ignore_invalid_rules { bail!("{} error(s) found", compiler.errors().len()); } + let ignored_rules: Vec<(String, String)> = compiler + .ignored_rules() + .map(|(rule_name, reason)| { + let reason_str = match reason { + IgnoredRuleReason::IgnoredModule(module) => { + format!("depends on ignored module `{module}`") + } + IgnoredRuleReason::IgnoredRule(parent_rule) => { + format!("depends on ignored rule `{parent_rule}`") + } + IgnoredRuleReason::CompileError(err) => { + format!("error: {}", err.title()) + } + }; + (rule_name.to_string(), reason_str) + }) + .collect(); + let rules = compiler.build(); - Ok(rules) + Ok((rules, ignored_rules)) } struct CompileState { diff --git a/cli/src/commands/scan.rs b/cli/src/commands/scan.rs index 3810940bd..fa1589cfc 100644 --- a/cli/src/commands/scan.rs +++ b/cli/src/commands/scan.rs @@ -253,7 +253,7 @@ pub fn exec_scan(args: &ArgMatches, config: &Config) -> anyhow::Result<()> { rules } else { - compile_rules(rules_path, args, config)? + compile_rules(rules_path, args, config)?.0 }; let rules_ref = &rules; diff --git a/cli/src/tests/compile.rs b/cli/src/tests/compile.rs new file mode 100644 index 000000000..b90abca26 --- /dev/null +++ b/cli/src/tests/compile.rs @@ -0,0 +1,57 @@ +use assert_cmd::{Command, cargo_bin}; +use assert_fs::TempDir; +use assert_fs::prelude::*; +use predicates::prelude::*; + +#[test] +fn ignore_invalid_rules() { + let temp_dir = TempDir::new().unwrap(); + let yar_file = temp_dir.child("test.yar"); + let yarc_file = temp_dir.child("test.yarc"); + + yar_file + .write_str( + r#" + rule valid_rule { + condition: true + } + rule invalid_rule { + condition: undefined_var == 1 + } + "#, + ) + .unwrap(); + + // Without --ignore-invalid-rules, compilation should fail early. + Command::new(cargo_bin!("yr")) + .arg("compile") + .arg("-o") + .arg(yarc_file.path()) + .arg(yar_file.path()) + .assert() + .failure() + .stderr(predicate::str::contains("1 error(s) found")); + + // With --ignore-invalid-rules, valid rules are compiled to destination file + // and ignored rules are listed in the message, completing with success. + Command::new(cargo_bin!("yr")) + .arg("compile") + .arg("--ignore-invalid-rules") + .arg("-o") + .arg(yarc_file.path()) + .arg(yar_file.path()) + .assert() + .success() + .stderr(predicate::str::contains("the following rules were ignored:")) + .stderr(predicate::str::contains("invalid_rule")); + + // Verify that the compiled rules file was created and contains the valid rule. + Command::new(cargo_bin!("yr")) + .arg("scan") + .arg("--compiled-rules") + .arg(yarc_file.path()) + .arg("src/tests/testdata/dummy.file") + .assert() + .success() + .stdout(predicate::str::contains("valid_rule")); +} diff --git a/cli/src/tests/mod.rs b/cli/src/tests/mod.rs index ce96b93ff..79142d0fd 100644 --- a/cli/src/tests/mod.rs +++ b/cli/src/tests/mod.rs @@ -1,4 +1,5 @@ mod check; +mod compile; #[cfg(feature = "debug-cmd")] mod debug; mod deps; diff --git a/cli/src/tests/scan.rs b/cli/src/tests/scan.rs index 44bcde3dc..d01ef8f05 100644 --- a/cli/src/tests/scan.rs +++ b/cli/src/tests/scan.rs @@ -451,3 +451,41 @@ fn cpu_limit() { .assert() .success(); } + +#[test] +fn ignore_invalid_rules() { + let temp_dir = TempDir::new().unwrap(); + let yar_file = temp_dir.child("test.yar"); + + yar_file + .write_str( + r#" + rule valid_rule { + condition: true + } + rule invalid_rule { + condition: undefined_var == 1 + } + "#, + ) + .unwrap(); + + // Without --ignore-invalid-rules, scan should fail on compile error. + Command::new(cargo_bin!("yr")) + .arg("scan") + .arg(yar_file.path()) + .arg("src/tests/testdata/dummy.file") + .assert() + .failure() + .stderr(predicate::str::contains("1 error(s) found")); + + // With --ignore-invalid-rules, scan should succeed and match valid_rule. + Command::new(cargo_bin!("yr")) + .arg("scan") + .arg("--ignore-invalid-rules") + .arg(yar_file.path()) + .arg("src/tests/testdata/dummy.file") + .assert() + .success() + .stdout(predicate::str::contains("valid_rule")); +} diff --git a/lib/src/compiler/mod.rs b/lib/src/compiler/mod.rs index ecc2d2178..18a9c6e38 100644 --- a/lib/src/compiler/mod.rs +++ b/lib/src/compiler/mod.rs @@ -82,6 +82,63 @@ pub mod linters; pub mod warnings; pub mod wsh; +/// The reason why a rule was ignored during compilation. +#[derive(Debug, PartialEq, Eq)] +pub enum IgnoredRuleReason<'a> { + /// The rule was ignored because it depends on a module that was ignored + /// with [`Compiler::ignore_module`]. Contains the name of the ignored + /// module. + IgnoredModule(&'a str), + /// The rule was ignored because it depends on another rule that was + /// ignored. Contains the name of the ignored rule it depends on. + IgnoredRule(&'a str), + /// The rule was ignored because of a compilation error. Contains a + /// reference to the error that caused compilation to fail. + CompileError(&'a CompileError), +} + +/// Internal version of [`IgnoredRuleReason`]. +/// +/// This is the version that is stored in the compiler. +#[derive(Debug, PartialEq, Eq)] +pub(crate) enum IgnoredRuleReasonInternal { + IgnoredModule(String), + IgnoredRule(String), + CompileError(usize), +} + +/// Iterator that yields rules ignored during compilation. +pub struct IgnoredRules<'a> { + iter: std::slice::Iter<'a, (String, IgnoredRuleReasonInternal)>, + errors: &'a [CompileError], +} + +impl<'a> Iterator for IgnoredRules<'a> { + type Item = (&'a str, IgnoredRuleReason<'a>); + + fn next(&mut self) -> Option { + let (rule_name, reason) = self.iter.next()?; + let reason = match reason { + IgnoredRuleReasonInternal::IgnoredModule(module) => { + IgnoredRuleReason::IgnoredModule(module.as_str()) + } + IgnoredRuleReasonInternal::IgnoredRule(rule_name) => { + IgnoredRuleReason::IgnoredRule(rule_name.as_str()) + } + IgnoredRuleReasonInternal::CompileError(err_idx) => { + IgnoredRuleReason::CompileError(&self.errors[*err_idx]) + } + }; + Some((rule_name, reason)) + } +} + +impl ExactSizeIterator for IgnoredRules<'_> { + fn len(&self) -> usize { + self.iter.len() + } +} + /// A structure that describes some YARA source code. /// /// This structure contains a `&str` pointing to the code itself, and an @@ -404,10 +461,14 @@ pub struct Compiler<'a> { /// if the banned module is imported. banned_modules: FxHashMap, + /// Vector containing the names of the rules that were ignored during + /// compilation, along with the reason why they were ignored. + ignored_rules: Vec<(String, IgnoredRuleReasonInternal)>, + /// Keys in this map are the name of rules that will be ignored because they /// depend on unsupported modules, either directly or indirectly. Values are /// the names of the unsupported modules they depend on. - ignored_rules: FxHashMap, + rules_depending_on_unsupported_modules: FxHashMap, /// Structure where each field corresponds to a global identifier or a module /// imported by the rules. For fields corresponding to modules, the value is @@ -512,7 +573,8 @@ impl<'a> Compiler<'a> { imported_modules: Vec::new(), ignored_modules: FxHashSet::default(), banned_modules: FxHashMap::default(), - ignored_rules: FxHashMap::default(), + ignored_rules: Vec::new(), + rules_depending_on_unsupported_modules: FxHashMap::default(), filesize_bounds: FxHashMap::default(), header_constraints: FxHashMap::default(), root_struct: Struct::new().make_root(), @@ -762,7 +824,7 @@ impl<'a> Compiler<'a> { ident_id: self.ident_pool.get_or_intern(namespace), symbols: self.symbol_table.push_new(), }; - self.ignored_rules.clear(); + self.rules_depending_on_unsupported_modules.clear(); self.wasm_mod.new_namespace(); self } @@ -1078,6 +1140,16 @@ impl<'a> Compiler<'a> { self.warnings.as_slice() } + /// Returns an iterator over the rules that were ignored during + /// compilation, along with the reason why they were ignored. + #[inline] + pub fn ignored_rules(&self) -> IgnoredRules<'_> { + IgnoredRules { + iter: self.ignored_rules.iter(), + errors: self.errors.as_slice(), + } + } + /// Emits a `.wasm` file with the WASM module generated by the compiler. /// /// This file can be inspected and converted to WASM text format by using @@ -1461,6 +1533,12 @@ impl Compiler<'_> { } ast::Item::Rule(rule) => { if let Err(err) = self.c_rule(rule) { + self.ignored_rules.push(( + rule.identifier.name.to_string(), + IgnoredRuleReasonInternal::CompileError( + self.errors.len(), + ), + )); self.errors.push(err); } } @@ -1640,56 +1718,82 @@ impl Compiler<'_> { } } - // In case of error, restore the compiler to the state it was before - // entering this function. Also, if the error is due to an unknown - // identifier, but the identifier is one of the unsupported modules, - // the error is tolerated and a warning is issued instead. let mut condition = match condition { Ok(condition) => condition, - Err(CompileError::UnknownIdentifier(unknown)) - if self.ignored_rules.contains_key(unknown.identifier()) - || self.ignored_modules.contains(unknown.identifier()) => - { + Err(err) => { + // In case of error, restore the compiler to the state it was + // before entering this function. self.restore_snapshot(snapshot); - if let Some(module_name) = - self.ignored_rules.get(unknown.identifier()) - { - self.warnings.add(|| { - warnings::IgnoredRule::build( - &self.report_builder, - module_name.clone(), + return match err { + // If the error is due to an unknown identifier, and the + // identifier is one of the ignored modules, the error + // is tolerated and a warning is issued instead. + CompileError::UnknownIdentifier(unknown) + if self + .ignored_modules + .contains(unknown.identifier()) => + { + self.warnings.add(|| { + IgnoredModule::build( + &self.report_builder, + unknown.identifier().to_string(), + unknown.identifier_location().clone(), + Some(format!( + "the whole rule `{}` will be ignored", + rule.identifier.name + )), + ) + }); + self.rules_depending_on_unsupported_modules.insert( rule.identifier.name.to_string(), - unknown.identifier_location().clone(), - ) - }); - self.ignored_rules.insert( - rule.identifier.name.to_string(), - module_name.clone(), - ); - } else { - self.warnings.add(|| { - warnings::IgnoredModule::build( - &self.report_builder, unknown.identifier().to_string(), - unknown.identifier_location().clone(), - Some(format!( - "the whole rule `{}` will be ignored", - rule.identifier.name - )), - ) - }); - self.ignored_rules.insert( - rule.identifier.name.to_string(), - unknown.identifier().to_string(), - ); - } + ); + self.ignored_rules.push(( + rule.identifier.name.to_string(), + IgnoredRuleReasonInternal::IgnoredModule( + unknown.identifier().to_string(), + ), + )); - return Ok(()); - } - Err(err) => { - self.restore_snapshot(snapshot); - return Err(err); + Ok(()) + } + // If the unknown identifier corresponds to one of the rules + // that depends directly or indirectly on an ignored module, + // the error is tolerated and a warning is issued instead. + CompileError::UnknownIdentifier(unknown) => { + if let Some(unsupported_module) = self + .rules_depending_on_unsupported_modules + .get(unknown.identifier()) + { + self.warnings.add(|| { + IgnoredRule::build( + &self.report_builder, + unsupported_module.clone(), + rule.identifier.name.to_string(), + unknown.identifier_location().clone(), + ) + }); + self.rules_depending_on_unsupported_modules + .insert( + rule.identifier.name.to_string(), + unsupported_module.clone(), + ); + self.ignored_rules.push(( + rule.identifier.name.to_string(), + IgnoredRuleReasonInternal::IgnoredRule( + unknown.identifier().to_string(), + ), + )); + + Ok(()) + } else { + Err(CompileError::UnknownIdentifier(unknown)) + } + } + // Any other kind of error is not tolerated. + _ => Err(err), + }; } }; diff --git a/lib/src/compiler/tests/mod.rs b/lib/src/compiler/tests/mod.rs index 634d5bd19..b56a3135d 100644 --- a/lib/src/compiler/tests/mod.rs +++ b/lib/src/compiler/tests/mod.rs @@ -5,7 +5,7 @@ use std::mem::size_of; use pretty_assertions::assert_eq; use serde_json::json; -use crate::compiler::{SubPattern, VarStack, linters}; +use crate::compiler::{IgnoredRuleReason, SubPattern, VarStack, linters}; use crate::errors::{SerializationError, VariableError}; use crate::types::Type; use crate::{Compiler, Rules, Scanner, SourceCode, compile}; @@ -622,6 +622,16 @@ fn unsupported_modules() { ) .unwrap(); + let ignored: Vec<_> = compiler.ignored_rules().collect(); + assert_eq!( + ignored, + vec![ + ("ignored_1", IgnoredRuleReason::IgnoredModule("foo_module")), + ("ignored_2", IgnoredRuleReason::IgnoredRule("ignored_1")), + ("ignored_3", IgnoredRuleReason::IgnoredRule("ignored_2")), + ] + ); + let rules = compiler.build(); assert_eq!( @@ -634,6 +644,34 @@ fn unsupported_modules() { ); } +#[test] +fn test_ignored_rules() { + let mut compiler = Compiler::new(); + compiler.ignore_module("unsupported_mod"); + + let _ = compiler.add_source( + r#" + import "unsupported_mod" + + rule rule_ok { condition: true } + rule rule_ignored_module { condition: unsupported_mod.field == 1 } + rule rule_failed_compile { condition: undefined_symbol == 1 } + "#, + ); + + let ignored: Vec<_> = compiler.ignored_rules().collect(); + assert_eq!(ignored.len(), 2); + + assert_eq!(ignored[0].0, "rule_ignored_module"); + assert_eq!( + ignored[0].1, + IgnoredRuleReason::IgnoredModule("unsupported_mod") + ); + + assert_eq!(ignored[1].0, "rule_failed_compile"); + assert!(matches!(ignored[1].1, IgnoredRuleReason::CompileError(_))); +} + #[cfg(feature = "test_proto2-module")] #[test] fn banned_modules() { diff --git a/lib/src/lib.rs b/lib/src/lib.rs index b439124d7..a463d0ee8 100644 --- a/lib/src/lib.rs +++ b/lib/src/lib.rs @@ -46,6 +46,8 @@ assert_eq!(results.matching_rules().len(), 1); extern crate core; pub use compiler::Compiler; +pub use compiler::IgnoredRuleReason; +pub use compiler::IgnoredRules; pub use compiler::Patch; pub use compiler::Rules; pub use compiler::RulesIter;