Skip to content

Commit

Permalink
fix: issue when modules and global variables were used at the same time.
Browse files Browse the repository at this point in the history
The root is of the issue was that global variables and modules were kept in separate structures during compile time, but they are together in the same struct at scan time. This caused that field indexes where computed with respect to either one structure or the other while compiling the rules, but at scan time these field numbers were not valid.
  • Loading branch information
plusvic committed Nov 22, 2023
1 parent 3b20cb1 commit f5f0602
Show file tree
Hide file tree
Showing 6 changed files with 58 additions and 31 deletions.
36 changes: 16 additions & 20 deletions yara-x/src/compiler/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -226,13 +226,10 @@ pub struct Compiler<'a> {
/// the [`IdentId`] corresponding to the module's identifier.
imported_modules: Vec<IdentId>,

/// Structure where each field corresponds to a module imported by the
/// rules. The value of each field is the structure that describes the
/// module.
modules_struct: Struct,

/// Structure where each field corresponds to some global identifier.
globals_struct: Struct,
/// Structure where each field corresponds to a global identifier or a module
/// imported by the rules. For fields corresponding to modules, the value is
/// is the structure that describes the module.
root_struct: Struct,

/// Warnings generated while compiling the rules.
warnings: Vec<Warning>,
Expand Down Expand Up @@ -302,8 +299,7 @@ impl<'a> Compiler<'a> {
atoms: Vec::new(),
re_code: Vec::new(),
imported_modules: Vec::new(),
modules_struct: Struct::new(),
globals_struct: Struct::new(),
root_struct: Struct::new(),
report_builder: ReportBuilder::new(),
lit_pool: BStringPool::new(),
regexp_pool: StringPool::new(),
Expand Down Expand Up @@ -382,15 +378,15 @@ impl<'a> Compiler<'a> {
let var: Variable = value.try_into()?;
let type_value: TypeValue = var.into();

if self.globals_struct.add_field(ident, type_value.clone()).is_some() {
if self.root_struct.add_field(ident, type_value.clone()).is_some() {
return Err(VariableError::AlreadyExists(ident.to_string()));
}

self.global_symbols.borrow_mut().insert(
ident,
Symbol::new(
type_value,
SymbolKind::FieldIndex(self.globals_struct.index_of(ident)),
SymbolKind::FieldIndex(self.root_struct.index_of(ident)),
),
);

Expand Down Expand Up @@ -481,7 +477,7 @@ impl<'a> Compiler<'a> {
// to `Arc`, as the root cause that prevents `Struct` from being `Send`
// is the use of `Rc` in `TypeValue`.
let serialized_globals = bincode::DefaultOptions::new()
.serialize(&self.globals_struct)
.serialize(&self.root_struct)
.expect("failed to serialize global variables");

let mut rules = Rules {
Expand Down Expand Up @@ -637,7 +633,7 @@ impl<'a> Compiler<'a> {
///
/// This functions checks if the module actually exists, and if so, it
/// creates a new field with the same name than the module in the
/// top-level structure `self.modules_struct` that contains all the
/// top-level structure `self.root_struct` that contains all the
/// imported modules. This field is created only if it don't exist yet.
fn import_module(&mut self, import: &Import) -> Result<(), CompileError> {
let module_name = import.module_name.as_str();
Expand All @@ -656,9 +652,9 @@ impl<'a> Compiler<'a> {
// Yes, module exists.
let module = module.unwrap();

// The module was already added to `self.modules_struct` and
// The module was already added to `self.globals_struct` and
// `self.imported_modules`, nothing more to do.
if self.modules_struct.has_field(module_name) {
if self.root_struct.has_field(module_name) {
return Ok(());
}

Expand Down Expand Up @@ -721,7 +717,7 @@ impl<'a> Compiler<'a> {
// modules. This struct contains all modules imported, from
// all namespaces. Panic if the module was already in the struct.
if self
.modules_struct
.root_struct
.add_field(module_name, TypeValue::Struct(Rc::new(module_struct)))
.is_some()
{
Expand Down Expand Up @@ -1476,7 +1472,7 @@ impl<'a> Compiler<'a> {

fn c_imports(&mut self, imports: &[Import]) -> Result<(), CompileError> {
for import in imports {
// Import the module. This updates `self.modules_struct` if
// Import the module. This updates `self.root_struct` if
// necessary.
self.import_module(import)?;

Expand All @@ -1491,14 +1487,14 @@ impl<'a> Compiler<'a> {
module_name,
Symbol::new(
// At this point the module must be found in
// `self.modules_struct`.
self.modules_struct
// `self.root_struct`.
self.root_struct
.field_by_name(module_name)
.unwrap()
.type_value
.clone(),
SymbolKind::FieldIndex(
self.modules_struct.index_of(module_name),
self.root_struct.index_of(module_name),
),
),
);
Expand Down
21 changes: 21 additions & 0 deletions yara-x/src/compiler/tests/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -476,3 +476,24 @@ fn import_modules() {
.add_source(r#"import "test_proto2" rule bar {condition: test_proto2.int32_zero == 0}"#)
.is_ok());
}

#[cfg(feature = "test_proto2-module")]
#[test]
fn issue() {
let mut compiler = Compiler::new();

compiler
.define_global("global_bool", false)
.unwrap()
.add_source(
r#"import "test_proto2" rule foo {
condition:
for all s in test_proto2.array_string: (s != "foo")
}"#,
)
.unwrap();

let rules = compiler.build();
let mut scanner = Scanner::new(&rules);
let _ = scanner.scan(b"foobar").unwrap();
}
2 changes: 1 addition & 1 deletion yara-x/src/scanner/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,7 @@ impl ScanContext<'_> {
.get(rule.namespace_ident_id)
.unwrap();

info!("Started rule evaluation: {}:{}", rule_name, rule_namespace);
info!("Started rule evaluation: {}:{}", rule_namespace, rule_name);
}

/// Called during the scan process when a global rule didn't match.
Expand Down
6 changes: 4 additions & 2 deletions yara-x/src/types/func.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use itertools::Itertools;
use serde::{Deserialize, Serialize};
use std::cmp::Ordering;

use crate::types::{TypeValue, Value};
Expand Down Expand Up @@ -45,6 +46,7 @@ use crate::types::{TypeValue, Value};
/// foo() -> Option<f32> -> foo@@fu
/// foo() -> Option<(f64,f64)> -> foo@@ffu
/// ```
#[derive(Serialize, Deserialize)]
pub struct MangledFnName(String);

impl MangledFnName {
Expand Down Expand Up @@ -110,7 +112,7 @@ where
///
/// YARA modules allow function overloading, therefore functions can have the
/// same name but different arguments.
#[derive(Clone)]
#[derive(Clone, Serialize, Deserialize)]
pub struct FuncSignature {
pub mangled_name: MangledFnName,
pub args: Vec<TypeValue>,
Expand Down Expand Up @@ -147,7 +149,7 @@ impl From<String> for FuncSignature {
}
}

#[derive(Clone)]
#[derive(Clone, Serialize, Deserialize)]
pub struct Func {
signatures: Vec<FuncSignature>,
}
Expand Down
11 changes: 5 additions & 6 deletions yara-x/src/types/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -69,11 +69,13 @@ impl From<Type> for ValType {
/// Contains information about a value.
#[derive(Clone, Serialize, Deserialize, PartialEq)]
pub enum Value<T> {
/// Constant value. The value is known and it can not change at runtime.
/// Constant value. The value is known at compile time and it cannot
/// change at runtime.
Const(T),
/// Variable value. The value is known, but it can change at runtime.
/// Variable value. The value is known at compile time, but it can change
/// at runtime.
Var(T),
/// Unknown value.
/// The value is unknown at compile time.
Unknown,
}

Expand Down Expand Up @@ -164,9 +166,6 @@ pub enum TypeValue {
Struct(Rc<Struct>),
Array(Rc<Array>),
Map(Rc<Map>),

// A TypeValue that contains a function is not serialized.
#[serde(skip)]
Func(Rc<Func>),
}

Expand Down
13 changes: 11 additions & 2 deletions yara-x/src/wasm/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -833,9 +833,18 @@ fn lookup_field(
} else {
*field_index
};
let field =
structure.field_by_index(field_index as usize).unwrap();

let field = structure
.field_by_index(field_index as usize)
.unwrap_or_else(|| {
panic!(
"expecting field with index {} in {:#?}",
field_index, structure
)
});

final_field = Some(field);

if let TypeValue::Struct(s) = &field.type_value {
structure = s
}
Expand Down

0 comments on commit f5f0602

Please sign in to comment.