From 3f1b2e3a2444117082a67b18f007b4f6facc959f Mon Sep 17 00:00:00 2001 From: Philipp Eder Date: Fri, 22 Nov 2024 15:49:01 +0000 Subject: [PATCH] Fix: double forking isn't possible Given: ``` set_kb_item(name: "port", value: 1); set_kb_item(name: "port", value: 2); set_kb_item(name: "port", value: 3); set_kb_item(name: "port", value: 4); set_kb_item(name: "port", value: 5); set_kb_item(name: "host", value: "a"); set_kb_item(name: "host", value: "b"); p = get_kb_item("port"); h = get_kb_item("host"); display(h, ":", p); ``` it should print every combination of host and port. This is done by changing the skip_values from an optional to vector and not removing it to allow multiple lookups within one statements. --- rust/src/nasl/interpreter/call.rs | 149 +++++++++++++++++++---- rust/src/nasl/interpreter/interpreter.rs | 88 ++++++++----- 2 files changed, 181 insertions(+), 56 deletions(-) diff --git a/rust/src/nasl/interpreter/call.rs b/rust/src/nasl/interpreter/call.rs index f57a05ecf..6c6845bb8 100644 --- a/rust/src/nasl/interpreter/call.rs +++ b/rust/src/nasl/interpreter/call.rs @@ -41,36 +41,49 @@ impl<'a> Interpreter<'a> { ); self.register_mut().create_root_child(named); let result = match self.ctxconfigs.nasl_fn_execute(name, self.register()).await { - Some(r) => { - if let Ok(NaslValue::Fork(mut x)) = r { - Ok(if let Some(r) = x.pop() { - // this is a proposal for the case that the caller is immediately executing - // if not the position needs to be reset - if self.index == 0 { - let position = self.position().current_init_statement(); - for i in x { - tracing::trace!(return_value=?i, return_position=?self.position(), interpreter_position=?position, "creating interpreter instance" ); - self.run_specific.push(RunSpecific { - register: self.register().clone(), - position: position.clone(), - skip_until_return: Some((self.position().clone(), i)), - }); - } + Some(Ok(NaslValue::Fork(x))) if self.index == 0 && !x.is_empty() => { + let mut additional = Vec::with_capacity(x.len() - 1); + let root_pos = self.run_specific[0].position.clone(); + + for (vi, v) in x.iter().enumerate() { + for (rsi, rs) in self.run_specific.iter_mut().enumerate() { + let mut pos = root_pos.clone(); + // needs to be reduced because a previous down statement enhanced the number + pos.reduce_last(); + + if vi == 0 { + rs.skip_until_return.push((pos, v.clone())); } else { - tracing::trace!( - index = self.index, - "we only allow expanding of executions (fork) on root instance" - ); + let position = pos.current_init_statement(); + let mut skip_until_return = rs + .skip_until_return + .iter() + .filter(|(p, _)| p != &pos) + .cloned() + .collect::>(); + skip_until_return.push((pos.clone(), v.clone())); + tracing::trace!(run_specific_index=rsi, value_index=vi, value=?v, ?pos, ?skip_until_return, ?rs.skip_until_return, "new fork"); + + additional.push(RunSpecific { + register: rs.register.clone(), + position: position.clone(), + skip_until_return, + }); } - tracing::trace!(return_value=?r, "returning interpreter instance" ); - r - } else { - NaslValue::Null - }) - } else { - r.map_err(|x| FunctionError::new(name, x).into()) + } } + self.run_specific.extend(additional); + Ok(x[0].clone()) + } + + Some(Ok(NaslValue::Fork(x))) if self.index == 0 && x.is_empty() => { + Ok(NaslValue::Null) + } + + Some(Ok(NaslValue::Fork(_))) => { + unreachable!("NaslValue::Fork must only occur on root instance, all other cases should return a value within run_specific") } + Some(r) => r.map_err(|x| FunctionError::new(name, x).into()), None => { let found = self .register() @@ -117,4 +130,88 @@ mod tests { t.ok("test(a: 1);", 1); t.ok("test();", 0); } + + #[test] + #[tracing_test::traced_test] + fn multiple_forks() { + let mut t = TestBuilder::default(); + t.run_all( + r#" +set_kb_item(name: "port", value: 1); +set_kb_item(name: "port", value: 2); +set_kb_item(name: "host", value: "a"); +set_kb_item(name: "host", value: "b"); +get_kb_item("port"); +get_kb_item("host"); +"#, + ); + + assert_eq!(t.results().len(), 10); + let results: Vec<_> = t + .results() + .into_iter() + .skip(4) + .filter_map(|x| x.ok()) + .collect(); + + assert_eq!( + results, + vec![ + 1.into(), + 2.into(), + "a".into(), + "a".into(), + "b".into(), + "b".into(), + ] + ); + } + #[test] + #[tracing_test::traced_test] + fn empty_fork() { + let mut t = TestBuilder::default(); + t.run_all( + r#" +get_kb_item("port") + ":" + get_kb_item("host"); +"#, + ); + + let results: Vec<_> = t + .results() + .into_iter() + .filter_map(|x| x.ok()) + .collect(); + + assert_eq!( + results, + vec!["\0:\0".into()] + ); + } + + #[test] + #[tracing_test::traced_test] + fn multiple_forks_on_one_line() { + let mut t = TestBuilder::default(); + t.run_all( + r#" +set_kb_item(name: "port", value: 1); +set_kb_item(name: "port", value: 2); +set_kb_item(name: "host", value: "a"); +set_kb_item(name: "host", value: "b"); +get_kb_item("port") + ":" + get_kb_item("host"); +"#, + ); + + let results: Vec<_> = t + .results() + .into_iter() + .skip(4) + .filter_map(|x| x.ok()) + .collect(); + + assert_eq!( + results, + vec!["1:a".into(), "2:a".into(), "1:b".into(), "2:b".into(),] + ); + } } diff --git a/rust/src/nasl/interpreter/interpreter.rs b/rust/src/nasl/interpreter/interpreter.rs index e0d14f01e..bcdcb0e7a 100644 --- a/rust/src/nasl/interpreter/interpreter.rs +++ b/rust/src/nasl/interpreter/interpreter.rs @@ -25,6 +25,20 @@ pub(crate) struct Position { index: Vec, } +impl std::fmt::Display for Position { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!( + f, + "{}", + self.index + .iter() + .map(|x| x.to_string()) + .collect::>() + .join(".") + ) + } +} + impl Position { pub fn new(index: usize) -> Self { Self { index: vec![index] } @@ -34,8 +48,20 @@ impl Position { self.index.push(0); } + pub fn reduce_last(&mut self) { + if let Some(last) = self.index.last_mut() { + if *last > 0 { + *last -= 1; + } + } + } + pub fn down(&mut self) -> Option { - self.index.pop() + let result = self.index.pop(); + if let Some(last) = self.index.last_mut() { + *last += 1; + } + result } pub fn current_init_statement(&self) -> Self { @@ -57,7 +83,7 @@ impl Position { pub(crate) struct RunSpecific { pub(crate) register: Register, pub(crate) position: Position, - pub(crate) skip_until_return: Option<(Position, NaslValue)>, + pub(crate) skip_until_return: Vec<(Position, NaslValue)>, } /// Used to interpret a Statement @@ -78,7 +104,7 @@ impl<'a> Interpreter<'a> { let root_run = RunSpecific { register, position: Position::new(0), - skip_until_return: None, + skip_until_return: Vec::new(), }; Interpreter { run_specific: vec![root_run], @@ -165,9 +191,6 @@ impl<'a> Interpreter<'a> { stmt: &Statement, max_attempts: usize, ) -> InterpretResult { - if let Some(last) = self.position_mut().index.last_mut() { - *last += 1; - } self.index = 0; self.retry_resolve(stmt, max_attempts).await } @@ -206,28 +229,37 @@ impl<'a> Interpreter<'a> { } } - /// Interprets a Statement - pub(crate) async fn resolve(&mut self, statement: &Statement) -> InterpretResult { - self.position_mut().up(); - tracing::trace!(position=?self.position(), statement=statement.to_string(), "executing"); - // On a fork statement run we skip until the root index is reached. Between the root index - // of the return value and the position of the return value the interpretation is - // continued. This is done because the client just executes higher statements. - if let Some((cp, rv)) = &self.skip_until_return() { - tracing::trace!(check_position=?cp); + //// Checks for skip_until_return and returns the value if the current position is in the list + /// if the root index is smaller than the current position it will return None this is done to + /// prevent unnecessary statement execution and has to be seen as guardian functionality. + fn may_return_value(&mut self) -> Option { + for (cp, value) in self.skip_until_return().iter() { if self.position().root_index() < cp.root_index() { - tracing::trace!("skip execution"); - self.position_mut().down(); - return Ok(NaslValue::Null); + return Some(NaslValue::Null); } if cp == self.position() { - tracing::trace!(return=?rv, "skip execution and returning"); - let rv = rv.clone(); - self.set_skip_until_return(None); - self.position_mut().down(); - return Ok(rv); + return Some(value.clone()); } } + None + } + + /// Interprets a Statement + pub(crate) async fn resolve(&mut self, statement: &Statement) -> InterpretResult { + self.position_mut().up(); + let span = tracing::span!(tracing::Level::WARN, "resolve", + statement=statement.to_string(), + index=self.index, + position=%self.position(), + run_specific_len=self.run_specific.len(), + skipped_value_pos=?self.skip_until_return(), ); + let _enter = span.enter(); + if let Some(val) = self.may_return_value() { + tracing::trace!(returns=?val, "skipped" ); + self.position_mut().down(); + return Ok(val); + } + tracing::trace!("executing"); let results = { match statement.kind() { @@ -339,13 +371,8 @@ impl<'a> Interpreter<'a> { rs.register = val; } - pub(crate) fn set_skip_until_return(&mut self, val: Option<(Position, NaslValue)>) { - let rs = &mut self.run_specific[self.index]; - rs.skip_until_return = val; - } - - pub(crate) fn skip_until_return(&self) -> Option<&(Position, NaslValue)> { - self.run_specific[self.index].skip_until_return.as_ref() + pub(crate) fn skip_until_return(&self) -> &[(Position, NaslValue)] { + &self.run_specific[self.index].skip_until_return } async fn resolve_exit(&mut self, statement: &Statement) -> Result { @@ -439,4 +466,5 @@ impl<'a> Interpreter<'a> { ), } } + }