Merge pull request #2248 in SNORT/snort3 from ~SELYSENK/snort3:coverity to master

Squashed commit of the following:

commit 35d120f022eb0a2596a02255a5fc0f6b4996444c
Author: Serhii Lysenko <selysenk@cisco.com>
Date:   Fri Jun 5 07:13:53 2020 -0400

    snort2lua: fix issues found by Coverity scans

    Add missing member initializations. Fix typos. Add missing checks for
    return values. Restore ostream flags.

    Fix OOB memory access in DataApi::expand_vars() and in
    Converter::parse_file().

diff --git a/tools/snort2lua/data/dt_data.cc b/tools/snort2lua/data/dt_data.cc
index c319b723caac1cf59127ae77a10cc7fb16eca9f8..17673972c376814a4846577eea513a4437e435e9 100644 (file)
--- a/tools/snort2lua/data/dt_data.cc
+++ b/tools/snort2lua/data/dt_data.cc
@@ -18,22 +18,24 @@
 // dt_data.cc author Josh Rosenbaum <jrosenba@cisco.com>
 
 #include "dt_data.h"
-#include "helpers/s2l_util.h"
+
+#include <algorithm>
+#include <cstring>
 #include <iostream>
 #include <sstream>
-#include <cstring>
 
 #include "data/data_types/dt_table.h"
 #include "data/data_types/dt_var.h"
 #include "data/data_types/dt_comment.h"
 #include "data/data_types/dt_rule.h"
 #include "data/data_types/dt_include.h"
+#include "helpers/s2l_util.h"
 
 DataApi::PrintMode DataApi::mode = DataApi::PrintMode::DEFAULT;
 std::size_t DataApi::dev_warnings = 0;
 std::size_t DataApi::errors_count = 0;
 
-DataApi::DataApi() : curr_data_bad(false)
+DataApi::DataApi()
 {
     comments = new Comments(start_comments, 0,
         Comments::CommentType::MULTI_LINE);
@@ -157,11 +159,11 @@ std::string DataApi::expand_vars(const std::string& string)
                     if (strlen(p) >= 2)
                     {
                         varmodifier = *(p + 1);
-                        std::strncpy(varaux, p + 2, sizeof(varaux));
+                        std::strncpy(varaux, p + 2, sizeof(varaux) - 1);
                     }
                 }
                 else
-                    std::strncpy(varname, rawvarname, sizeof(varname));
+                    std::strncpy(varname, rawvarname, sizeof(varname) - 1);
 
                 std::memset((char*)varbuffer, 0, sizeof(varbuffer));
 
@@ -345,13 +347,7 @@ void DataApi::swap_conf_data(std::vector<Variable*>& new_vars,
 {
     vars.swap(new_vars);
     includes.swap(new_includes);
-
-    Comments* tmp = new_comments;
-    new_comments = comments;
-    comments = tmp;
-
-    tmp = new_unsupported;
-    new_unsupported = unsupported;
-    unsupported = tmp;
+    std::swap(comments, new_comments);
+    std::swap(unsupported, new_unsupported);
 }
 

diff --git a/tools/snort2lua/data/dt_data.h b/tools/snort2lua/data/dt_data.h
index ff6f7f1740da93e99621a5586ad3861b6b3fbae8..3d774d65f19cc691d4ab53dd448896c912084fc7 100644 (file)
--- a/tools/snort2lua/data/dt_data.h
+++ b/tools/snort2lua/data/dt_data.h
@@ -145,9 +145,9 @@ private:
     Comments* errors;
     Comments* unsupported;
 
-    bool curr_data_bad;  // keep track whether current 'conversion' is already bad
-    const std::string* current_file;
-    unsigned current_line;
+    bool curr_data_bad = false;  // keep track whether current 'conversion' is already bad
+    const std::string* current_file = nullptr;
+    unsigned current_line = 0;
 
     std::string get_file_line();
 };

diff --git a/tools/snort2lua/helpers/converter.cc b/tools/snort2lua/helpers/converter.cc
index a263d06d6eb0d2c2bde5cf43f663beeb7cac03bf..c0b05934b7a1ef10b5c08c1ff6a0bb616336bb47 100644 (file)
--- a/tools/snort2lua/helpers/converter.cc
+++ b/tools/snort2lua/helpers/converter.cc
@@ -226,6 +226,7 @@ int Converter::parse_file(
         // same criteria used for rtrim
         // http://en.cppreference.com/w/cpp/string/byte/isspace
         std::size_t first_non_white_char = tmp.find_first_not_of(" \f\n\r\t\v");
+        std::size_t last_non_space = tmp.find_last_not_of(' ');
 
         bool comment = (tmp[first_non_white_char] == '#') or (tmp[first_non_white_char] == ';');
         bool commented_rule = tmp.substr(0, 7) == "# alert";
@@ -245,7 +246,8 @@ int Converter::parse_file(
             }
             data_api.add_comment(tmp);
         }
-        else if ( tmp[tmp.find_last_not_of(' ')] == '\\')
+        else if ( (last_non_space != std::string::npos) and
+            (tmp[last_non_space] == '\\') )
         {
             util::rtrim(tmp);
             tmp.pop_back();

diff --git a/tools/snort2lua/helpers/parse_cmd_line.cc b/tools/snort2lua/helpers/parse_cmd_line.cc
index 71477472d4b705b3097d1968e9f2759247857118..aca65c8831eb76e5ca6dd228931e36caba35451d 100644 (file)
--- a/tools/snort2lua/helpers/parse_cmd_line.cc
+++ b/tools/snort2lua/helpers/parse_cmd_line.cc
@@ -530,5 +530,6 @@ static void help_args(const char* /*pfx*/, const char* /*val*/)
         }
         ++p;
     }
+    std::cout << std::resetiosflags(std::ios::adjustfield);
 }
 } // namespace parser

diff --git a/tools/snort2lua/keyword_states/kws_attribute_table.cc b/tools/snort2lua/keyword_states/kws_attribute_table.cc
index 1d5caea7d0a568620d8bbb00f6793611454919ee..329d989006beaeb35209589fa89e4fb40455cd73 100644 (file)
--- a/tools/snort2lua/keyword_states/kws_attribute_table.cc
+++ b/tools/snort2lua/keyword_states/kws_attribute_table.cc
@@ -37,7 +37,7 @@ public:
     bool convert(std::istringstream& data) override;
 
 private:
-    std::istringstream* stream; // so I can call ld->failed_conversion
+    std::istringstream* stream = nullptr; // so I can call ld->failed_conversion
     std::unordered_map<std::string, std::string> attr_map;
     std::ifstream attr_file;
 

diff --git a/tools/snort2lua/preprocessor_states/pps_http_inspect_server.cc b/tools/snort2lua/preprocessor_states/pps_http_inspect_server.cc
index 1b6ba974f09837ba6207886909fba2dfda528c77..2952d674544cd48620e5f3b3e97e498154a0f3aa 100644 (file)
--- a/tools/snort2lua/preprocessor_states/pps_http_inspect_server.cc
+++ b/tools/snort2lua/preprocessor_states/pps_http_inspect_server.cc
@@ -329,7 +329,7 @@ bool HttpInspectServer::convert(std::istringstream& data_stream)
         else if (keyword == "profile")
             parse_deleted_option("profile", data_stream);
         else if ( keyword == "xff_headers" )
-            parse_bracketed_unsupported_list("xff_headers", data_stream);
+            tmpval = parse_bracketed_unsupported_list("xff_headers", data_stream);
         else
         {
             tmpval = false;

diff --git a/tools/snort2lua/preprocessor_states/pps_sfportscan.cc b/tools/snort2lua/preprocessor_states/pps_sfportscan.cc
index c7dd6a771af16ef49109c22fcb873fd3ee2af407..07d8010fd8c49849a7c050e321ecf7eae33cbc96 100644 (file)
--- a/tools/snort2lua/preprocessor_states/pps_sfportscan.cc
+++ b/tools/snort2lua/preprocessor_states/pps_sfportscan.cc
@@ -122,17 +122,17 @@ bool PortScan::convert(std::istringstream& data_stream)
             table_api.add_deleted_comment("logfile");
         }
         else if (keyword == "memcap")
-            tmpval = parse_option("memcap", data_stream) && retval;
+            tmpval = parse_option("memcap", data_stream);
 
         else if (keyword == "proto")
         {
             table_api.add_diff_option_comment("proto", "protos");
-            retval = parse_curly_bracket_list("protos", data_stream) && retval;
+            tmpval = parse_curly_bracket_list("protos", data_stream);
         }
         else if (keyword == "scan_type")
         {
             table_api.add_diff_option_comment("scan_type", "scan_types");
-            tmpval = parse_curly_bracket_list("scan_types", data_stream) && retval;
+            tmpval = parse_curly_bracket_list("scan_types", data_stream);
         }
         else
             tmpval = false;

diff --git a/tools/snort2lua/rule_states/rule_base64_decode.cc b/tools/snort2lua/rule_states/rule_base64_decode.cc
index 78da9998499d249bae73c8697ec192336b227dde..b06b6ced46519f4f569f2242e44f982b1b73459e 100644 (file)
--- a/tools/snort2lua/rule_states/rule_base64_decode.cc
+++ b/tools/snort2lua/rule_states/rule_base64_decode.cc
@@ -61,11 +61,10 @@ bool Base64Decode::convert(std::istringstream& data_stream)
             // since we still can't be sure if we passed the base64_decode buffer,
             // check the next option and ensure it matches
             std::istringstream arg_stream(args);
-            util::get_string(arg_stream, tmp, ", ");
-
-            if (tmp == "bytes" ||
+            if (util::get_string(arg_stream, tmp, ", ") &&
+                (tmp == "bytes" ||
                 tmp == "offset" ||
-                tmp == "relative")
+                tmp == "relative"))
             {
                 rule_api.add_option("base64_decode", args);
             }

diff --git a/tools/snort2lua/rule_states/rule_react.cc b/tools/snort2lua/rule_states/rule_react.cc
index 4fa669428da1bbe366a175166b7530d048ef81a6..8fb34304932ae77a555eba6219b66701c56d39bf 100644 (file)
--- a/tools/snort2lua/rule_states/rule_react.cc
+++ b/tools/snort2lua/rule_states/rule_react.cc
@@ -61,12 +61,11 @@ bool React::convert(std::istringstream& data_stream)
             // since we still can't be sure if we passed the resp buffer,
             // check the next option and ensure it matches
             std::istringstream arg_stream(args);
-            util::get_string(arg_stream, tmp, ",");
-
-            if (tmp == "msg" ||
+            if (util::get_string(arg_stream, tmp, ",") &&
+                (tmp == "msg" ||
                 tmp == "warn" ||
                 tmp == "block" ||
-                !tmp.compare(0, 5, "proxy"))
+                !tmp.compare(0, 5, "proxy")))
             {
                 // Now that we have confirmed this is a valid option, parse it!!
                 table_api.open_table("react");

diff --git a/tools/snort2lua/rule_states/rule_resp.cc b/tools/snort2lua/rule_states/rule_resp.cc
index d74a3c70e3cbe8db5a5b0bc80e599091dcf50730..ad57b554ab3ab4a2e35273d3a61e3148963ee240 100644 (file)
--- a/tools/snort2lua/rule_states/rule_resp.cc
+++ b/tools/snort2lua/rule_states/rule_resp.cc
@@ -69,9 +69,8 @@ bool Resp::convert(std::istringstream& data_stream)
             // since we still can't be sure if we passed the resp buffer,
             // check the next option and ensure it matches
             std::istringstream arg_stream(args);
-            util::get_string(arg_stream, tmp, ",");
-
-            if (tmp == "reset_dest" ||
+            if (util::get_string(arg_stream, tmp, ",") &&
+                (tmp == "reset_dest" ||
                 tmp == "reset_both" ||
                 tmp == "rst_snd" ||
                 tmp == "rst_rcv" ||
@@ -80,7 +79,7 @@ bool Resp::convert(std::istringstream& data_stream)
                 tmp == "icmp_host" ||
                 tmp == "icmp_all" ||
                 tmp == "reset_source" ||
-                tmp == "icmp_port")
+                tmp == "icmp_port"))
             {
                 // Now that we have confirmed this is a valid option, parse it!!
                 table_api.open_table("reject");