Skip to content

Commit 0d1763e

Browse files
committed
fix: logic error in block scanner that causes a use-after-free.
The `finish` method in the block scanner was triggering a call to `search_for_pattern` that acceded a block of memory that could be already de-allocated. The logic has been fixed to avoid these calls to `search_for_patterns`. Closes VirusTotal#548
1 parent aaadeb6 commit 0d1763e

5 files changed

Lines changed: 30 additions & 15 deletions

File tree

lib/src/compiler/emit.rs

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1038,13 +1038,11 @@ fn emit_lazy_call_to_search_for_patterns(
10381038
},
10391039
|_else| {
10401040
_else
1041-
// Call `search_for_patterns`.
1041+
// Call `search_for_patterns`. This function will set
1042+
// `pattern_search_done` to true before exiting.
10421043
.call(ctx.function_id(
10431044
wasm::export__search_for_patterns.mangled_name,
1044-
))
1045-
// Set `pattern_search_done` to true.
1046-
.i32_const(1)
1047-
.global_set(ctx.wasm_symbols.pattern_search_done);
1045+
));
10481046
},
10491047
);
10501048
let top = ctx.emit_search_for_pattern_stack.last_mut().unwrap();

lib/src/scanner/blocks.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -151,8 +151,12 @@ impl<'r> Scanner<'r> {
151151
let ctx = self.scan_context_mut();
152152

153153
ctx.scan_state = ScanState::ScanningBlock((base, data));
154+
155+
ctx.set_pattern_search_done(false);
154156
ctx.search_for_patterns()?;
155157

158+
ctx.scan_state = ScanState::Idle;
159+
156160
for (_, match_list) in ctx.pattern_matches.matches_per_pattern() {
157161
// Here we iterate the matches in order to gather snippets of data
158162
// from where the matches occurred. Notice however that we are only

lib/src/scanner/context.rs

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,9 @@ pub(crate) struct ScanContext<'r, 'd> {
7474
pub wasm_main_memory: Option<wasmtime::Memory>,
7575
/// WASM global variable that contains the value of `filesize`.
7676
pub wasm_filesize: Option<Global>,
77+
/// WASM global variable that contains a boolean that indicates if
78+
/// pattern search was done.
79+
pub wasm_pattern_search_done: Option<Global>,
7780
/// Map where keys are object handles and values are objects used during
7881
/// the evaluation of rule conditions. Handles are opaque integer values
7982
/// that can be passed to and received from WASM code. Each handle identify
@@ -357,7 +360,7 @@ impl ScanContext<'_, '_> {
357360
obj_ref
358361
}
359362

360-
/// Get the value of the global variable `filesize`.
363+
/// Gets the value of the global variable `filesize`.
361364
pub(crate) fn get_filesize(&mut self) -> i64 {
362365
self.wasm_filesize.unwrap().get(self.wasm_store_mut()).i64().unwrap()
363366
}
@@ -370,13 +373,22 @@ impl ScanContext<'_, '_> {
370373
.unwrap();
371374
}
372375

376+
/// Sets the value of the flag that indicates if the pattern search
377+
/// phase was already executed.
378+
pub(crate) fn set_pattern_search_done(&mut self, done: bool) {
379+
self.wasm_pattern_search_done
380+
.unwrap()
381+
.set(self.wasm_store_mut(), Val::I32(done as i32))
382+
.unwrap();
383+
}
384+
373385
/// Sets a timeout for scan operations.
374386
pub(crate) fn set_timeout(&mut self, timeout: Duration) -> &mut Self {
375387
self.scan_timeout = Some(timeout);
376388
self
377389
}
378390

379-
/// Invoke the main function, which evaluates the rules' conditions. It
391+
/// Invokes the main function, which evaluates the rules' conditions. It
380392
/// calls ScanContext::search_for_patterns (which does the Aho-Corasick
381393
/// scanning) only if necessary.
382394
///
@@ -1032,6 +1044,9 @@ impl ScanContext<'_, '_> {
10321044
_ => unreachable!(),
10331045
};
10341046

1047+
// Indicate that the pattern search phase was already done.
1048+
self.set_pattern_search_done(true);
1049+
10351050
#[cfg(any(feature = "rules-profiling", feature = "logging"))]
10361051
let scan_end = self.clock.raw();
10371052

@@ -1789,6 +1804,7 @@ pub fn create_wasm_store_and_ctx<'r>(
17891804
wasm_main_memory: None,
17901805
wasm_main_func: None,
17911806
wasm_filesize: None,
1807+
wasm_pattern_search_done: None,
17921808
module_outputs: FxHashMap::default(),
17931809
user_provided_module_outputs: FxHashMap::default(),
17941810
pattern_matches: PatternMatches::new(),
@@ -1915,6 +1931,7 @@ pub fn create_wasm_store_and_ctx<'r>(
19151931
ctx.wasm_main_memory = Some(main_memory);
19161932
ctx.wasm_main_func = Some(main_fn);
19171933
ctx.wasm_filesize = Some(filesize);
1934+
ctx.wasm_pattern_search_done = Some(pattern_search_done);
19181935

19191936
wasm_store
19201937
}

lib/src/scanner/mod.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -599,6 +599,9 @@ impl<'r> Scanner<'r> {
599599
// by the rules.
600600
ctx.user_provided_module_outputs.clear();
601601

602+
// Clear the flag that indicates that the search phase was done.
603+
ctx.set_pattern_search_done(false);
604+
602605
// Evaluate the conditions of every rule, this will call
603606
// `ScanContext::search_for_patterns` if necessary.
604607
ctx.eval_conditions()?;

lib/src/wasm/builder.rs

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -197,14 +197,7 @@ impl WasmModuleBuilder {
197197
);
198198

199199
// The main function receives no arguments and returns an I32.
200-
let mut main_func =
201-
FunctionBuilder::new(&mut module.types, &[], &[I32]);
202-
203-
// The first instructions in the main function initialize the global
204-
// variables `pattern_search_done`.
205-
main_func.func_body().i32_const(0);
206-
main_func.func_body().global_set(pattern_search_done);
207-
200+
let main_func = FunctionBuilder::new(&mut module.types, &[], &[I32]);
208201
let namespace_block = namespace_func.dangling_instr_seq(None).id();
209202

210203
Self {

0 commit comments

Comments
 (0)