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

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 2464b467244335ed18a98b71326086592a1f1045..8b5100c0b4b3b2f853aac5c785679be98abcb762 100644 (file)
--- 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 136a6dd6e074c5197fc6b4b8fb5f4a3db4607f8a..25ef1e5ccc0879b23d414ae8589b926d8a956d25 100644 (file)
--- 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 d9f0681b632df21802fcb2559c813a003719dbda..e4051693f9ad6a42b902505fd1799ce5771a5e5f 100644 (file)
--- 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 a3ee60b6eb2991bfc68cb5bc9d2b340847c6b38f..e15e1838e200675c2b5c33ffdf237845f4c0b428 100644 (file)
--- 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();
     }
 }