Skip to content

Commit

Permalink
feat: remove Arc used around module values
Browse files Browse the repository at this point in the history
Those were needed to fix ownership issues when evaluating module values
with operations chains. This has been fixed by moving the evaluation of
the expressions used in operations outside of the evaluation of those
operations.
  • Loading branch information
vthib committed Oct 22, 2023
1 parent a9842a8 commit 0d11cc2
Show file tree
Hide file tree
Showing 5 changed files with 14 additions and 21 deletions.
2 changes: 1 addition & 1 deletion boreal-cli/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -249,7 +249,7 @@ fn scan_file(scanner: &Scanner, path: &Path, options: ScanOptions) -> std::io::R
for (module_name, module_value) in res.module_values {
// A module value must be an object. Filter out empty ones, it means the module has not
// generated any values.
if let ModuleValue::Object(map) = &*module_value {
if let ModuleValue::Object(map) = &module_value {
if !map.is_empty() {
print!("{module_name}");
print_module_value(&module_value, 4);
Expand Down
22 changes: 9 additions & 13 deletions boreal/src/evaluator/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@
//! - `defined`
//!
//! For all of those, an undefined value is considered to be equivalent to a false boolean value.
use std::sync::Arc;
use std::time::Duration;

use crate::compiler::expression::{Expression, ForIterator, ForSelection, VariableIndex};
Expand Down Expand Up @@ -114,7 +113,7 @@ impl From<ExternalValue> for Value {
pub struct ScanData<'a> {
pub mem: &'a [u8],

pub module_values: Vec<(&'static str, Arc<ModuleValue>)>,
pub module_values: Vec<(&'static str, ModuleValue)>,

// Context used when calling module functions
module_ctx: ScanContext<'a>,
Expand Down Expand Up @@ -153,9 +152,7 @@ impl<'a> ScanData<'a> {
.map(|module| {
(
module.get_name(),
Arc::new(crate::module::Value::Object(
module.get_dynamic_values(&mut module_ctx),
)),
crate::module::Value::Object(module.get_dynamic_values(&mut module_ctx)),
)
})
.collect(),
Expand Down Expand Up @@ -222,7 +219,7 @@ struct Evaluator<'scan, 'rule> {
currently_selected_variable_index: Option<usize>,

// Stack of bounded identifiers to their integer values.
bounded_identifiers_stack: Vec<Arc<ModuleValue>>,
bounded_identifiers_stack: Vec<ModuleValue>,

// Data related only to the scan, independent of the rule.
scan_data: &'rule mut ScanData<'scan>,
Expand Down Expand Up @@ -845,7 +842,7 @@ impl Evaluator<'_, '_> {
match value {
ModuleValue::Array(array) => {
for value in array {
self.bounded_identifiers_stack.push(Arc::new(value));
self.bounded_identifiers_stack.push(value);
let v = self.evaluate_expr(body);
self.bounded_identifiers_stack.truncate(prev_stack_len);
let v = match v {
Expand All @@ -865,9 +862,8 @@ impl Evaluator<'_, '_> {
}
ModuleValue::Dictionary(dict) => {
for (key, value) in dict {
self.bounded_identifiers_stack
.push(Arc::new(ModuleValue::Bytes(key)));
self.bounded_identifiers_stack.push(Arc::new(value));
self.bounded_identifiers_stack.push(ModuleValue::Bytes(key));
self.bounded_identifiers_stack.push(value);
let v = self.evaluate_expr(body);
self.bounded_identifiers_stack.truncate(prev_stack_len);
let v = match v {
Expand Down Expand Up @@ -900,7 +896,7 @@ impl Evaluator<'_, '_> {

for value in from..=to {
self.bounded_identifiers_stack
.push(Arc::new(ModuleValue::Integer(value)));
.push(ModuleValue::Integer(value));
let v = self.evaluate_expr(body);
self.bounded_identifiers_stack.truncate(prev_stack_len);
let v = match v {
Expand All @@ -925,11 +921,11 @@ impl Evaluator<'_, '_> {
match self.evaluate_expr(expr)? {
Value::Integer(value) => {
self.bounded_identifiers_stack
.push(Arc::new(ModuleValue::Integer(value)));
.push(ModuleValue::Integer(value));
}
Value::Bytes(value) => {
self.bounded_identifiers_stack
.push(Arc::new(ModuleValue::Bytes(value)));
.push(ModuleValue::Bytes(value));
}
_ => return Err(PoisonKind::Undefined),
}
Expand Down
6 changes: 2 additions & 4 deletions boreal/src/evaluator/module.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
//! Provides methods to evaluate module values during scanning.
use std::iter::Peekable;
use std::slice::Iter;
use std::sync::Arc;

use crate::compiler::module::{
BoundedValueIndex, ModuleExpression, ModuleExpressionKind, ModuleOperations, ValueOperation,
Expand Down Expand Up @@ -43,9 +42,8 @@ pub(super) fn evaluate_expr(
.get(*index)
.ok_or(PoisonKind::Undefined)?,
};
let value = Arc::clone(value);

evaluate_ops(evaluator, &value, ops, expressions)
evaluate_ops(evaluator, value, ops, expressions)
}
ModuleExpressionKind::StaticFunction { fun } => {
let Some(ValueOperation::FunctionCall(nb_arguments)) = ops.next() else {
Expand All @@ -64,7 +62,7 @@ pub(super) fn evaluate_expr(
}

fn evaluate_ops(
evaluator: &mut Evaluator,
evaluator: &Evaluator,
mut value: &ModuleValue,
mut operations: Peekable<Iter<ValueOperation>>,
mut expressions: std::vec::IntoIter<Value>,
Expand Down
2 changes: 1 addition & 1 deletion boreal/src/scanner/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -485,7 +485,7 @@ pub struct ScanResult<'scanner> {
/// On-scan values of all modules used in the scanner.
///
/// First element is the module name, second one is the dynamic values produced by the module.
pub module_values: Vec<(&'static str, Arc<crate::module::Value>)>,
pub module_values: Vec<(&'static str, crate::module::Value)>,

/// Indicates if evaluation timed out.
///
Expand Down
3 changes: 1 addition & 2 deletions boreal/tests/it/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -562,7 +562,7 @@ pub fn compare_module_values_on_mem<M: Module>(
let scanner = compiler.into_scanner();

let res = scanner.scan_mem(mem);
let boreal_value = res
let mut boreal_value = res
.module_values
.into_iter()
.find_map(|(name, module_value)| {
Expand All @@ -575,7 +575,6 @@ pub fn compare_module_values_on_mem<M: Module>(
.unwrap();

// Enrich value using the static values, so that it can be compared with yara's
let mut boreal_value = Arc::try_unwrap(boreal_value).unwrap();
enrich_with_static_values(&mut boreal_value, module.get_static_values());

let c = yara::Compiler::new().unwrap();
Expand Down

0 comments on commit 0d11cc2

Please sign in to comment.