Merge pull request #2877 in SNORT/snort3 from ~DERAMADA/snort3:load_lua_exit to master

Squashed commit of the following:

commit 5a4ccf3e529c7f3b96b4330c0dad66a3d3d14d57
Author: Deepak Ramadass <deramada@cisco.com>
Date:   Thu Apr 29 16:25:03 2021 -0400

    shell: exit gracefully when sanbox lua is misconfigured

diff --git a/src/main/shell.cc b/src/main/shell.cc
index 9313dfcd29d1958a6524032ddec747c8c0ebc80d..cd2ce0b49e87a39ba27feee63b2bbed646513c05 100644 (file)
--- a/src/main/shell.cc
+++ b/src/main/shell.cc
@@ -302,43 +302,59 @@ static int get_line_number(lua_State* L)
 
 #endif
 
-void Shell::set_sandbox_env()
+bool Shell::set_sandbox_env()
 {
     lua_getglobal(lua, "sandbox_env");
 
     if ( lua_istable(lua, -1) )
     {
         if ( !lua_setfenv(lua, -2) )
-            FatalError("can't set sandbox environment\n");
+        {
+            ParseError("can't set sandbox environment\n");
+            return false;
+        }
     }
     else
-        FatalError("sandbox environment not defined\n");
+    {
+        ParseError("sandbox environment not defined\n");
+        return false;
+    }
+    return true;
 }
 
 bool Shell::load_lua_sandbox()
 {
-    if (lua_sandbox.empty())
-        return false;
-
     Lua::ManageStack ms(lua);
 
     LogMessage("Loading lua sandbox %s:\n", lua_sandbox.c_str());
     if ( luaL_loadfile(lua, lua_sandbox.c_str()) )
-        FatalError("can't load lua sandbox %s: %s\n", lua_sandbox.c_str(), lua_tostring(lua, -1));
+    {
+        ParseError("can't load lua sandbox %s: %s\n", lua_sandbox.c_str(), lua_tostring(lua, -1));
+        return false;
+    }
 
     if ( lua_pcall(lua, 0, 0, 0) )
-        FatalError("can't init lua sandbox %s: %s\n", lua_sandbox.c_str(), lua_tostring(lua, -1));
+    {
+        ParseError("can't init lua sandbox %s: %s\n", lua_sandbox.c_str(), lua_tostring(lua, -1));
+        return false;
+    }
     LogMessage("Finished %s:\n", lua_sandbox.c_str());
 
     lua_getglobal(lua, "sandbox_env");
     if ( !lua_istable(lua, -1) )
-        FatalError("sandbox_env table doesn't exist in %s: %s\n", lua_sandbox.c_str(),
+    {
+        ParseError("sandbox_env table doesn't exist in %s: %s\n", lua_sandbox.c_str(),
             lua_tostring(lua, -1));
+        return false;
+    }
 
     lua_getglobal(lua, "create_sandbox_env");
     if ( lua_pcall(lua, 0, 0, 0) != 0 )
-        FatalError("can't create sandbox environment %s: %s\n", lua_sandbox.c_str(),
+    {
+        ParseError("can't create sandbox environment %s: %s\n", lua_sandbox.c_str(),
             lua_tostring(lua, -1));
+        return false;
+    }
 
     return true;
 }
@@ -348,42 +364,52 @@ bool Shell::load_string(const char* s, bool load_in_sandbox, const char* message
     Lua::ManageStack ms(lua);
 
     if ( luaL_loadstring(lua, s) )
-        FatalError("can't load %s: %s\n", message, lua_tostring(lua, -1));
+    {
+        ParseError("can't load %s: %s\n", message, lua_tostring(lua, -1));
+        return false;
+    }
+
+    if ( load_in_sandbox && !set_sandbox_env() )
+        return false;
 
-    if ( load_in_sandbox )
-        set_sandbox_env();
 
     if ( lua_pcall(lua, 0, 0, 0) )
-        FatalError("can't init %s: %s\n", message, lua_tostring(lua, -1));
+    {
+        ParseError("can't init %s: %s\n", message, lua_tostring(lua, -1));
+        return false;
+    }
 
     return true;
 }
 
-bool Shell::load_config(const char* file, bool load_in_sandbox, bool is_fatal)
+bool Shell::load_config(const char* file, bool load_in_sandbox)
 {
     if ( load_in_sandbox )
     {
         ifstream in_file(file, ifstream::in);
         if (in_file.get() == 27 )
-            FatalError("bytecode is not allowed %s\n", file);
+        {
+            ParseError("bytecode is not allowed %s\n", file);
+            return false;
+        }
     }
 
     Lua::ManageStack ms(lua);
 
     if ( luaL_loadfile(lua, file) )
     {
-        if (is_fatal)
-            FatalError("can't load %s: %s\n", file, lua_tostring(lua, -1));
-        else
-            ParseError("can't load %s: %s\n", file, lua_tostring(lua, -1));
+        ParseError("can't load %s: %s\n", file, lua_tostring(lua, -1));
         return false;
     }
 
-    if ( load_in_sandbox )
-        set_sandbox_env();
+    if ( load_in_sandbox && !set_sandbox_env() )
+        return false;
 
     if ( lua_pcall(lua, 0, 0, 0) )
-        FatalError("can't init %s: %s\n", file, lua_tostring(lua, -1));
+    {
+        ParseError("can't init %s: %s\n", file, lua_tostring(lua, -1));
+        return false;
+    }
 
     return true;
 }
@@ -440,7 +466,7 @@ void Shell::set_overrides(Shell* sh)
     overrides += sh->overrides;
 }
 
-bool Shell::configure(SnortConfig* sc, bool is_fatal, bool is_root)
+bool Shell::configure(SnortConfig* sc, bool is_root)
 {
     assert(file.size());
     ModuleManager::set_config(sc);
@@ -463,7 +489,12 @@ bool Shell::configure(SnortConfig* sc, bool is_fatal, bool is_root)
         lua_setglobal(lua, "tweaks");
     }
 
-    bool load_in_sandbox = load_lua_sandbox();
+    bool load_in_sandbox = true;
+  
+    if ( lua_sandbox.empty() )
+        load_in_sandbox = false;
+    else if ( !load_lua_sandbox() )
+        return false;
 
     if ( load_defaults )
         load_string(ModuleManager::get_lua_coreinit(), load_in_sandbox, "coreinit");
@@ -481,10 +512,7 @@ bool Shell::configure(SnortConfig* sc, bool is_fatal, bool is_root)
 
     if ( !code )
     {
-        if ( is_fatal )
-            FatalError("can't find %s\n", file.c_str());
-        else
-            ParseError("can't find %s\n", file.c_str());
+        ParseError("can't find %s\n", file.c_str());
         return false;
     }
 
@@ -493,7 +521,7 @@ bool Shell::configure(SnortConfig* sc, bool is_fatal, bool is_root)
     current_shells.push(this);
 
     if ( !path.empty() and
-        !load_config(path.c_str(), load_in_sandbox, is_fatal) )
+        !load_config(path.c_str(), load_in_sandbox) )
     {
         current_shells.pop();
         return false;

diff --git a/src/main/shell.h b/src/main/shell.h
index abc0af8a77b6aa10c104076925a7bb226825cd0c..1c010b24ef1186e707aacb2be28191464824e666 100644 (file)
--- a/src/main/shell.h
+++ b/src/main/shell.h
@@ -53,7 +53,7 @@ public:
     void set_overrides(const char*);
     void set_overrides(Shell*);
 
-    bool configure(snort::SnortConfig*, bool is_fatal = true, bool is_root = false);
+    bool configure(snort::SnortConfig*, bool is_root = false);
     void install(const char*, const struct luaL_Reg*);
     void execute(const char*, std::string&);
 
@@ -118,9 +118,9 @@ private:
     void allowlist_update(const char* keyword, bool is_prefix);
 
     bool load_lua_sandbox();
-    void set_sandbox_env();
+    bool set_sandbox_env();
     bool load_string(const char* s, bool load_in_sandbox, const char* message);
-    bool load_config(const char* file, bool load_in_sandbox, bool is_fatal);
+    bool load_config(const char* file, bool load_in_sandbox);
 
 private:
     bool loaded;

diff --git a/src/main/snort.cc b/src/main/snort.cc
index 713a98252d28c4ddfdcb35ad18f501e471f74f5e..3f7c2c2c863991f64c939d59351d6d69c38cd41c 100644 (file)
--- a/src/main/snort.cc
+++ b/src/main/snort.cc
@@ -449,7 +449,7 @@ SnortConfig* Snort::get_reload_config(const char* fname, const char* plugin_path
     trim_heap();
 
     parser_init();
-    SnortConfig* sc = ParseSnortConf(snort_cmd_line_conf, fname, false);
+    SnortConfig* sc = ParseSnortConf(snort_cmd_line_conf, fname);
 
     if ( get_parse_errors() || ModuleManager::get_errors() || !sc->verify() )
     {
@@ -545,7 +545,7 @@ SnortConfig* Snort::get_updated_policy(
             !other_conf->trace_config->initialized;
 
         Shell sh = Shell(fname);
-        sh.configure(sc, false, true);
+        sh.configure(sc, true);
 
         if ( uninitialized_trace )
         {

diff --git a/src/parser/parser.cc b/src/parser/parser.cc
index a889183fc51ae119507a1e58ba612910cacbda7b..b6a524821e1f1e677b3e8bf572acf7021d061405 100644 (file)
--- a/src/parser/parser.cc
+++ b/src/parser/parser.cc
@@ -282,14 +282,14 @@ static RuleListNode* addNodeToOrderedList
     return ordered_list;
 }
 
-static bool parse_file(SnortConfig* sc, Shell* sh, bool is_fatal, bool is_root)
+static bool parse_file(SnortConfig* sc, Shell* sh, bool is_root)
 {
     const char* fname = sh->get_file();
 
     if ( !fname || !*fname )
         return false;
 
-    bool success = sh->configure(sc, is_fatal, is_root);
+    bool success = sh->configure(sc, is_root);
 
     return success;
 }
@@ -315,7 +315,7 @@ void parser_term(SnortConfig*)
     ruleIndexMap = nullptr;
 }
 
-SnortConfig* ParseSnortConf(const SnortConfig* cmd_line_conf, const char* fname, bool is_fatal)
+SnortConfig* ParseSnortConf(const SnortConfig* cmd_line_conf, const char* fname)
 {
     const SnortConfig* current_conf = SnortConfig::get_conf();
     SnortConfig* sc = new SnortConfig(current_conf->proto_ref);
@@ -365,7 +365,7 @@ SnortConfig* ParseSnortConf(const SnortConfig* cmd_line_conf, const char* fname,
         Shell::set_config_output(shell_output);
         set_policies(sc, sh);
 
-        if (!parse_file(sc, sh, is_fatal, (i == 0)))
+        if (!parse_file(sc, sh, (i == 0)))
         {
             parse_file_failed = true;
             break;

diff --git a/src/parser/parser.h b/src/parser/parser.h
index 9cd30dcec6e031dbe5ae4303210683277db8f97f..6c7ded60ab3a144b0f2bff0ac54c108f6b053a83 100644 (file)
--- a/src/parser/parser.h
+++ b/src/parser/parser.h
@@ -42,8 +42,7 @@ void push_parse_location(
 void pop_parse_location();
 void inc_parse_position();
 
-snort::SnortConfig* ParseSnortConf(const snort::SnortConfig*, const char* fname = nullptr,
-    bool is_fatal = true);
+snort::SnortConfig* ParseSnortConf(const snort::SnortConfig*, const char* fname = nullptr);
 void ParseRules(snort::SnortConfig*);
 void ParseRulesFinish(snort::SnortConfig*);
 void ShowPolicyStats(const snort::SnortConfig*);

diff --git a/src/target_based/host_attributes.cc b/src/target_based/host_attributes.cc
index f250d79e7d2a5d2dc4527eab8bf596fcaeb69ef3..24e989868a1fa5df67d8246e089662ab7f044b2c 100644 (file)
--- a/src/target_based/host_attributes.cc
+++ b/src/target_based/host_attributes.cc
@@ -142,7 +142,7 @@ bool HostAttributesManager::load_hosts_file(snort::SnortConfig* sc, const char*
     next_cache = new HostAttributesSharedCache(sc->max_attribute_hosts);
 
     Shell sh(fname);
-    if ( sh.configure(sc, false, true) )
+    if ( sh.configure(sc, true) )
     {
         activate(sc);
         return true;