feat: implement basic linting (#282)

Introduces the `Compiler::add_linter` method that allows adding linting rules to the compiler and produce additional warnings. Two linters were implemented: one that verifies that rule names match a given regular expression, and another one that validates metadata entries according to a custom predicate. However, the mechanism is flexible enough and more linters can be implemented in the future. The CLI's `check` command uses the new API for validating rule names and metadata entries according to the rules specified by the configuration file.
VirusTotal · Jan 30, 2025 · ddd6f38 · ddd6f38
1 parent 2788dce
commit ddd6f38
Show file tree

Hide file tree

Showing 19 changed files with 907 additions and 55 deletions.
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/Cargo.toml b/Cargo.toml
@@ -95,6 +95,8 @@ serde_json = "1.0.133"
 sha1 = "0.10.6"
 sha2 = "0.10.8"
 smallvec = "1.13.2"
+strum = "0.26.3"
+strum_macros = "0.26.4"
 thiserror = "2.0.3"
 # Using tlsh-fixed instead of tlsh because tlsh-fixed includes a fix for this
 # issue: https://github.com/1crcbl/tlsh-rs/issues/2.

diff --git a/cli/Cargo.toml b/cli/Cargo.toml
@@ -58,6 +58,8 @@ protobuf = { workspace = true }
 protobuf-json-mapping = { workspace = true }
 serde_json = { workspace = true, features = ["preserve_order"] }
 serde = { workspace = true, features = ["derive"] }
+strum = { workspace = true }
+strum_macros = { workspace = true }
 yansi = { workspace = true }
 yara-x = { workspace = true, features = ["parallel-compilation"] }
 yara-x-parser = { workspace = true }
@@ -69,7 +71,6 @@ colored_json = "5.0.0"
 crossbeam = "0.8.4"
 crossterm = "0.28.1"
 encoding_rs = "0.8.35"
-strum_macros = "0.26.4"
 superconsole = "0.2.0"
 wild = "2.2.1"
 

diff --git a/cli/src/commands/check.rs b/cli/src/commands/check.rs
@@ -2,16 +2,17 @@ use std::path::PathBuf;
 use std::sync::atomic::{AtomicUsize, Ordering};
 use std::{fs, io};
 
+use crate::config::{CheckConfig, MetaValueType};
+use crate::walk::Message;
+use crate::{help, walk};
 use anyhow::Context;
 use clap::{arg, value_parser, ArgAction, ArgMatches, Command};
 use crossterm::tty::IsTty;
 use superconsole::{Component, Line, Lines, Span};
 use yansi::Color::{Green, Red, Yellow};
 use yansi::Paint;
-use yara_x::SourceCode;
-
-use crate::walk::Message;
-use crate::{help, walk};
+use yara_x::{linters, SourceCode};
+use yara_x_parser::ast::MetaValue;
 
 pub fn check() -> Command {
     super::command("check")
@@ -47,7 +48,22 @@ pub fn check() -> Command {
         )
 }
 
-pub fn exec_check(args: &ArgMatches) -> anyhow::Result<()> {
+fn is_sha256(s: &str) -> bool {
+    s.len() == 64 && s.chars().all(|c| c.is_ascii_hexdigit())
+}
+
+fn is_sha1(s: &str) -> bool {
+    s.len() == 40 && s.chars().all(|c| c.is_ascii_hexdigit())
+}
+
+fn is_md5(s: &str) -> bool {
+    s.len() == 32 && s.chars().all(|c| c.is_ascii_hexdigit())
+}
+
+pub fn exec_check(
+    args: &ArgMatches,
+    config: CheckConfig,
+) -> anyhow::Result<()> {
     let rules_path = args.get_one::<PathBuf>("RULES_PATH").unwrap();
     let max_depth = args.get_one::<u16>("max-depth");
     let filters = args.get_many::<String>("filter");
@@ -90,6 +106,74 @@ pub fn exec_check(args: &ArgMatches) -> anyhow::Result<()> {
             let mut lines = Vec::new();
             let mut compiler = yara_x::Compiler::new();
 
+            for (identifier, config) in config.metadata.iter() {
+                let mut linter =
+                    linters::metadata(identifier).required(config.required);
+
+                match config.ty {
+                    MetaValueType::String => {
+                        linter = linter.validator(
+                            |meta| {
+                                matches!(
+                                    meta.value,
+                                    MetaValue::String(_) | MetaValue::Bytes(_)
+                                )
+                            },
+                            "metadata value must be a string",
+                        );
+                    }
+                    MetaValueType::Integer => {
+                        linter = linter.validator(
+                            |meta| matches!(meta.value, MetaValue::Integer(_)),
+                            "metadata value must be a string",
+                        );
+                    }
+                    MetaValueType::Float => {
+                        linter = linter.validator(
+                            |meta| matches!(meta.value, MetaValue::Float(_)),
+                            "metadata value must be a float",
+                        );
+                    }
+                    MetaValueType::Bool => {
+                        linter = linter.validator(
+                            |meta| matches!(meta.value, MetaValue::Bool(_)),
+                            "metadata value must be a bool",
+                        );
+                    }
+                    MetaValueType::Sha256 => {
+                        linter = linter.validator(
+                            |meta| matches!(meta.value, MetaValue::String(s) if is_sha256(s)),
+                            "metadata value must be a SHA-256",
+                        );
+                    }
+                    MetaValueType::Sha1 => {
+                        linter = linter.validator(
+                            |meta| matches!(meta.value, MetaValue::String(s) if is_sha1(s)),
+                            "metadata value must be a SHA-1",
+                        );
+                    }
+                    MetaValueType::MD5 => {
+                        linter = linter.validator(
+                            |meta| matches!(meta.value, MetaValue::String(s) if is_md5(s)),
+                            "metadata value must be a MD5",
+                        );
+                    }
+                    MetaValueType::Hash => {
+                        linter = linter.validator(
+                            |meta| matches!(meta.value, MetaValue::String(s)
+                                if is_md5(s) || is_sha1(s) || is_sha256(s)),
+                            "metadata value must be a MD5, SHA-1 or SHA-256",
+                        );
+                    }
+                }
+
+                compiler.add_linter(linter);
+            }
+
+            if let Some(re) = &config.rule_name_regexp {
+                compiler.add_linter(linters::rule_name(re)?);
+            }
+
             compiler.colorize_errors(io::stdout().is_tty());
 
             match compiler.add_source(src) {
@@ -111,8 +195,8 @@ pub fn exec_check(args: &ArgMatches) -> anyhow::Result<()> {
                             "WARN".paint(Yellow).bold(),
                             file_path.display()
                         ));
-                        for warning in compiler.warnings().iter() {
-                            lines.push(warning.to_string());
+                        for warning in compiler.warnings() {
+                            eprintln!("{}", warning);
                         }
                     }
                 }

diff --git a/cli/src/commands/fmt.rs b/cli/src/commands/fmt.rs
@@ -6,8 +6,8 @@ use std::{fs, io, process};
 use clap::{arg, value_parser, ArgAction, ArgMatches, Command};
 use yara_x_fmt::Formatter;
 
-use crate::config::{load_config_from_file, FormatConfig};
-use crate::help::{CONFIG_FILE, FMT_CHECK_MODE};
+use crate::config::FormatConfig;
+use crate::help::FMT_CHECK_MODE;
 
 pub fn fmt() -> Command {
     super::command("fmt")
@@ -20,26 +20,14 @@ pub fn fmt() -> Command {
                 .action(ArgAction::Append),
         )
         .arg(arg!(-c --check  "Run in 'check' mode").long_help(FMT_CHECK_MODE))
-        .arg(
-            arg!(-C --config <CONFIG_FILE> "Config file")
-                .value_parser(value_parser!(PathBuf))
-                .long_help(CONFIG_FILE),
-        )
 }
 
 pub fn exec_fmt(
     args: &ArgMatches,
-    main_config: FormatConfig,
+    config: FormatConfig,
 ) -> anyhow::Result<()> {
     let files = args.get_many::<PathBuf>("FILE").unwrap();
     let check = args.get_flag("check");
-    let config_file = args.get_one::<PathBuf>("config");
-
-    let config: FormatConfig = if config_file.is_some() {
-        load_config_from_file(config_file.unwrap())?.fmt
-    } else {
-        main_config
-    };
 
     let formatter = Formatter::new()
         .align_metadata(config.meta.align_values)

diff --git a/cli/src/commands/mod.rs b/cli/src/commands/mod.rs
@@ -23,13 +23,13 @@ use std::io::stdout;
 use std::path::PathBuf;
 
 use anyhow::{anyhow, bail, Context};
-use clap::{command, crate_authors, ArgMatches, Command};
+use clap::{arg, command, crate_authors, ArgMatches, Command};
 use crossterm::tty::IsTty;
 use superconsole::{Component, Line, Lines, Span, SuperConsole};
 use yansi::Color::Green;
 use yansi::Paint;
 
-use crate::{commands, APP_HELP_TEMPLATE};
+use crate::{commands, help, APP_HELP_TEMPLATE};
 use yara_x::{Compiler, Rules, SourceCode};
 
 use crate::walk::Walker;
@@ -49,6 +49,11 @@ pub fn cli() -> Command {
     command!()
         .author(crate_authors!("\n")) // requires `cargo` feature
         .arg_required_else_help(true)
+        .arg(
+            arg!(-C --config <CONFIG_FILE> "Config file")
+                .value_parser(existing_path_parser)
+                .long_help(help::CONFIG_FILE),
+        )
         .help_template(APP_HELP_TEMPLATE)
         .subcommands(vec![
             commands::scan(),
@@ -133,6 +138,16 @@ fn path_with_namespace_parser(
     }
 }
 
+/// Parses a path and makes sure that it exists.
+fn existing_path_parser(input: &str) -> Result<PathBuf, anyhow::Error> {
+    let path = PathBuf::from(input);
+    if path.try_exists()? {
+        Ok(path)
+    } else {
+        Err(anyhow!("file not found"))
+    }
+}
+
 pub fn create_compiler(
     external_vars: Option<Vec<(String, serde_json::Value)>>,
     args: &ArgMatches,