From: Lukasz Czarnik -X (lczarnik - SOFTSERVE INC at Cisco) <lczarnik@cisco.com>
Date: Fri, 16 Feb 2024 19:12:29 +0000 (+0000)
Subject: Pull request #4166: control: fix crash in update_scratch and ctrlcon block related... 
X-Git-Tag: 3.1.81.0~2
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=82fdf5bdd0e1f5222809e62cd4e2b134f501aada;p=thirdparty%2Fsnort3.git

Pull request #4166: control: fix crash in update_scratch and ctrlcon block related issues

Merge in SNORT/snort3 from ~LCZARNIK/snort3:scratch_crash to master

Squashed commit of the following:

commit 2efd39cac108297f9bfa6b7ca768bd0ae9c2ed10
Author: Lukasz Czarnik <lczarnik@cisco.com>
Date:   Tue Jan 23 08:54:09 2024 -0500

    control: Adds counting to ctrlcon blocked to allow for nested commands
---

diff --git a/src/control/control.cc b/src/control/control.cc
index 2464b4672..8b5100c0b 100644
--- a/src/control/control.cc
+++ b/src/control/control.cc
@@ -67,9 +67,6 @@ ControlConn::~ControlConn()
 
 void ControlConn::shutdown()
 {
-    if (blocked)
-        blocked = false;
-
     if (is_closed())
         return;
     if (!local)
@@ -189,7 +186,7 @@ int ControlConn::execute_commands()
 
 void ControlConn::block()
 {
-    blocked = true;
+    blocked++;
 }
 
 void ControlConn::remove()
@@ -211,7 +208,7 @@ void ControlConn::unblock()
 {
     if (blocked)
     {
-        blocked = false;
+        blocked--;
         execute_commands();
         if (!blocked && !show_prompt())
             shutdown();
diff --git a/src/control/control.h b/src/control/control.h
index 136a6dd6e..25ef1e5cc 100644
--- a/src/control/control.h
+++ b/src/control/control.h
@@ -81,8 +81,8 @@ private:
     std::string next_command;
     class Shell *shell;
     int fd;
+    int blocked = 0; // a number of commands blocking the channel
     bool local = false;
-    bool blocked = false; //block any new commands from executing before current command in control connection is complete
     bool removed = false;
     time_t touched;
 
diff --git a/src/control/control_mgmt.cc b/src/control/control_mgmt.cc
index d9f0681b6..e4051693f 100644
--- a/src/control/control_mgmt.cc
+++ b/src/control/control_mgmt.cc
@@ -543,3 +543,76 @@ bool ControlMgmt::service_users()
     return (serviced > 0);
 }
 
+
+// -----------------------------------------------------------------------------
+// unit tests
+// -----------------------------------------------------------------------------
+
+
+#ifdef UNIT_TEST
+#include "catch/snort_catch.h"
+#include "main/ac_shell_cmd.h"
+
+class ACExample : public AnalyzerCommand
+{
+    bool execute(Analyzer &, void **) override { return true; }
+    const char * stringify() override { return "ACExample"; }
+    ~ACExample() override {}
+};
+
+TEST_CASE("Do not delete ctrlcon if its in use by another ACShellCmd")
+{
+    int pipefd[2];
+    pipe(pipefd);
+
+    ControlConn* ctrlcon = new ControlConn(pipefd[1], false);
+
+    auto iter = controls.insert({pipefd[1], ctrlcon});
+
+    ACShellCmd* acshell1 = new ACShellCmd(ctrlcon, new ACExample());
+    ACShellCmd* acshell2 = new ACShellCmd(ctrlcon, new ACExample());
+
+    delete_control(iter.first);
+
+    delete acshell1;
+
+    CHECK((ctrlcon->is_blocked() == true));
+    CHECK((ctrlcon->is_closed() == false));
+
+    delete acshell2;
+
+    close(pipefd[0]);
+    close(pipefd[1]);
+
+};
+
+TEST_CASE("Do not unblock ctrlcon if its in use by another ACShellCmd")
+{
+    int pipefd[2];
+    pipe(pipefd);
+
+    ControlConn* ctrlcon = new ControlConn(pipefd[1], false);
+
+    auto iter = controls.insert({pipefd[1], ctrlcon});
+
+    ACShellCmd* acshell1 = new ACShellCmd(ctrlcon, new ACExample());
+    ACShellCmd* acshell2 = new ACShellCmd(ctrlcon, new ACExample());
+
+    CHECK((ctrlcon->is_blocked() == true));
+
+    delete acshell1;
+
+    CHECK((ctrlcon->is_blocked() == true));
+
+    delete acshell2;
+
+    CHECK((ctrlcon->is_blocked() == false));
+
+    delete_control(iter.first);
+
+    close(pipefd[0]);
+    close(pipefd[1]);
+
+};
+
+#endif
diff --git a/src/main/ac_shell_cmd.cc b/src/main/ac_shell_cmd.cc
index a3ee60b6e..e15e1838e 100644
--- a/src/main/ac_shell_cmd.cc
+++ b/src/main/ac_shell_cmd.cc
@@ -49,9 +49,8 @@ ACShellCmd::~ACShellCmd()
 
     if (ctrlcon)
     {
-        if (ctrlcon->is_removed())
+        ctrlcon->unblock();
+        if (ctrlcon->is_removed() and !ctrlcon->is_blocked())
             delete ctrlcon;
-        else
-            ctrlcon->unblock();
     }
 }