From: Steve Chew (stechew) <stechew@cisco.com>
Date: Mon, 17 May 2021 19:47:08 +0000 (+0000)
Subject: Merge pull request #2877 in SNORT/snort3 from ~DERAMADA/snort3:load_lua_exit to master
X-Git-Tag: 3.1.5.0~5
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=693a9b9ec97bb90a119a84480b685b252e3ed125;p=thirdparty%2Fsnort3.git

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 9313dfcd2..cd2ce0b49 100644
--- 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 abc0af8a7..1c010b24e 100644
--- 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 713a98252..3f7c2c2c8 100644
--- 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 a889183fc..b6a524821 100644
--- 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 9cd30dcec..6c7ded60a 100644
--- 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 f250d79e7..24e989868 100644
--- 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;