Fix: nasl-cli/openvasd: possible stackoverflow on End::Continue

When a NASL script gets executed a lot of parens that includes SyntaxError then nasl-cli may get stuck in an recursive loop that overflows the stack therefore it was reported with the severity `VSS:4.0/AV:L/AC:L/AT:N/PR:N/UI:N/VC:N/VI:L/VA:H/SC:N/SI:L/SA:H`. To fix it Lexer rembers it's current depth and when it get higher than the defined MAX_DEPTH it returns an SyntaxError of kind MaxRecursion. If it is based on an previous error then the iterator implementation of Lexer simulates an EoF to prevent multiple false positive errors to be reported.
greenbone · Nov 28, 2023 · a4cbe5e · a4cbe5e
1 parent c482c2f
commit a4cbe5e
Show file tree

Hide file tree

Showing 5 changed files with 78 additions and 5 deletions.
diff --git a/rust/nasl-syntax/src/error.rs b/rust/nasl-syntax/src/error.rs
@@ -24,6 +24,8 @@ pub enum ErrorKind {
     MissingSemicolon(Statement),
     /// An token is unclosed
     UnclosedStatement(Statement),
+    /// Maximal recursion depth reached. Simplify NASL code.
+    MaxRecursionDepth(u8),
     /// The cursor is already at the end but that is not expected
     EoF,
     /// An IO Error occurred while loading a NASL file
@@ -50,6 +52,7 @@ impl SyntaxError {
             ErrorKind::UnclosedStatement(s) => s.as_token(),
             ErrorKind::EoF => None,
             ErrorKind::IOError(_) => None,
+            ErrorKind::MaxRecursionDepth(_) => None,
         }
     }
 }
@@ -177,6 +180,25 @@ macro_rules! unexpected_end {
     }};
 }
 
+/// Creates an maximal recursion depth reached error.
+///
+/// To prevent stack overflows the Lexer veriefies it's depth and returns an error.
+///
+/// # Examples
+///
+/// Basic usage:
+/// ```rust
+/// use nasl_syntax::max_recursion;
+/// max_recursion!(255);
+/// ```
+#[macro_export]
+macro_rules! max_recursion {
+    ($reason:expr) => {{
+        use $crate::syntax_error;
+        use $crate::ErrorKind;
+        syntax_error!(ErrorKind::MaxRecursionDepth($reason))
+    }};
+}
 impl fmt::Display for SyntaxError {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         write!(f, "{}", self.kind)
@@ -194,6 +216,10 @@ impl fmt::Display for ErrorKind {
             ErrorKind::MissingSemicolon(stmt) => write!(f, "missing semicolon: {stmt}"),
             ErrorKind::EoF => write!(f, "end of file."),
             ErrorKind::IOError(kind) => write!(f, "IOError: {kind}"),
+            ErrorKind::MaxRecursionDepth(max) => write!(
+                f,
+                "Maximal recursion depth of {max} reached, the NASL script is too complex."
+            ),
         }
     }
 }

diff --git a/rust/nasl-syntax/src/keyword_extension.rs b/rust/nasl-syntax/src/keyword_extension.rs
@@ -39,9 +39,9 @@ impl<'a> Lexer<'a> {
     fn parse_if(&mut self, kw: Token) -> Result<(End, Statement), SyntaxError> {
         let ptoken = self.token().ok_or_else(|| unexpected_end!("if parsing"))?;
         let condition = match ptoken.category() {
-            Category::LeftParen => self.parse_paren(ptoken.clone()),
-            _ => Err(unexpected_token!(ptoken.clone())),
-        }?
+            Category::LeftParen => self.parse_paren(ptoken.clone())?,
+            _ => return Err(unexpected_token!(ptoken.clone())),
+        }
         .as_returnable_or_err()?;
         let (end, body) = self.statement(0, &|cat| cat == &Category::Semicolon)?;
         if end == End::Continue {

diff --git a/rust/nasl-syntax/src/lexer.rs b/rust/nasl-syntax/src/lexer.rs
@@ -8,6 +8,7 @@ use std::ops::Not;
 use crate::{
     error::SyntaxError,
     infix_extension::Infix,
+    max_recursion,
     operation::Operation,
     postfix_extension::Postfix,
     prefix_extension::Prefix,
@@ -20,6 +21,11 @@ pub struct Lexer<'a> {
     // TODO: change to iterator of Token instead of Tokenizer
     // to allopw statements of a Vec
     tokenizer: Tokenizer<'a>,
+
+    // is the current depth call within a statement call. The current
+    // implementation relies that the iterator implementation resets depth to 0
+    // after a statement, or error, has been returned.
+    depth: u8,
 }
 
 #[derive(Clone, Debug, PartialEq, Eq)]
@@ -28,6 +34,9 @@ pub enum End {
     Continue,
 }
 
+/// Is the maximum depth allowed within one continuous statement call.
+const MAX_DEPTH: u8 = 42;
+
 impl End {
     pub fn is_done(&self) -> bool {
         match self {
@@ -55,7 +64,8 @@ impl Not for End {
 impl<'a> Lexer<'a> {
     /// Creates a Lexer
     pub fn new(tokenizer: Tokenizer<'a>) -> Lexer<'a> {
-        Lexer { tokenizer }
+        let depth = 0;
+        Lexer { tokenizer, depth }
     }
 
     /// Returns next token of tokenizer
@@ -93,6 +103,10 @@ impl<'a> Lexer<'a> {
         min_binding_power: u8,
         abort: &impl Fn(&Category) -> bool,
     ) -> Result<(End, Statement), SyntaxError> {
+        self.depth += 1;
+        if self.depth >= MAX_DEPTH {
+            return Err(max_recursion!(MAX_DEPTH));
+        }
         // reset unhandled_token when min_bp is 0
         let (state, mut left) = self
             .token()
@@ -101,21 +115,26 @@ impl<'a> Lexer<'a> {
                     return Err(unexpected_token!(token));
                 }
                 if abort(token.category()) {
+                    self.depth = 0;
                     return Ok((End::Done(token.clone()), Statement::NoOp(Some(token))));
                 }
                 self.prefix_statement(token, abort)
             })
             .unwrap_or(Ok((End::Done(Token::unexpected_none()), Statement::EoF)))?;
         match state {
             End::Continue => {}
-            end => return Ok((end, left)),
+            end => {
+                self.depth = 0;
+                return Ok((end, left));
+            }
         }
 
         let mut end_statement = End::Continue;
         while let Some(token) = self.peek() {
             if abort(token.category()) {
                 self.token();
                 end_statement = End::Done(token.clone());
+                self.depth = 0;
                 break;
             }
             let op =
@@ -128,6 +147,7 @@ impl<'a> Lexer<'a> {
                 self.token();
                 left = stmt;
                 if let End::Done(cat) = end {
+                    self.depth = 0;
                     end_statement = End::Done(cat);
                     break;
                 }
@@ -142,6 +162,7 @@ impl<'a> Lexer<'a> {
                 let (end, nl) = self.infix_statement(op, token, left, abort)?;
                 left = nl;
                 if let End::Done(cat) = end {
+                    self.depth = 0;
                     end_statement = End::Done(cat);
                     break;
                 } else {
@@ -162,6 +183,11 @@ impl<'a> Iterator for Lexer<'a> {
 
     fn next(&mut self) -> Option<Self::Item> {
         let result = self.statement(0, &|cat| cat == &Category::Semicolon);
+        // simulate eof if end::continue is stuck in a recursive loop
+        if self.depth >= MAX_DEPTH {
+            return None;
+        }
+
         match result {
             Ok((_, Statement::EoF)) => None,
             Ok((End::Done(_), stmt)) => Some(Ok(stmt)),

diff --git a/rust/nasl-syntax/tests/crash-recursion-depth.nasl b/rust/nasl-syntax/tests/crash-recursion-depth.nasl
@@ -0,0 +1 @@
+i~f&((((((((((((((((((((((((((((((((+(((((((((((re(((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((~f&((((((((((((((((((((((((((((((((+(((((((((((re(((((((((((((((((((((((((((((((((((((((((((((((((((((~f&((((((((((((((((((((((((((((((((+(((((((((((re((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((~f&((((((((((((((((((((((((((((((((+(((((((((((re(((((((((((,i
diff --git a/rust/nasl-syntax/tests/missing_input_validation.rs b/rust/nasl-syntax/tests/missing_input_validation.rs
@@ -0,0 +1,20 @@
+// SPDX-FileCopyrightText: 2023 Greenbone AG
+//
+// SPDX-License-Identifier: GPL-2.0-or-later
+
+#[cfg(test)]
+mod test {
+
+    use nasl_syntax::{logger::NaslLogger, parse};
+
+    #[test]
+    fn validate_recursion_depth_to_prevent_stackoverflow() {
+        // Reported by Anon, VSS:4.0/AV:L/AC:L/AT:N/PR:N/UI:N/VC:N/VI:L/VA:H/SC:N/SI:L/SA:H
+        // Crash due to depth limit on recursion.
+        let code = include_str!("crash-recursion-depth.nasl");
+        assert_eq!(code.len(), 587);
+        let result = nasl_syntax::parse(code).collect::<Vec<_>>();
+        assert_eq!(result.len(), 1);
+        assert!(result[0].is_err())
+    }
+}