From: Bhagya Tholpady (bbantwal) Date: Wed, 17 Jun 2020 14:57:10 +0000 (+0000) Subject: Merge pull request #2248 in SNORT/snort3 from ~SELYSENK/snort3:coverity to master X-Git-Tag: 3.0.1-5~3 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=dbd64b10a6dab6e95580d2b9e46f840c108338a4;p=thirdparty%2Fsnort3.git Merge pull request #2248 in SNORT/snort3 from ~SELYSENK/snort3:coverity to master Squashed commit of the following: commit 35d120f022eb0a2596a02255a5fc0f6b4996444c Author: Serhii Lysenko 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 c319b723c..17673972c 100644 --- a/tools/snort2lua/data/dt_data.cc +++ b/tools/snort2lua/data/dt_data.cc @@ -18,22 +18,24 @@ // dt_data.cc author Josh Rosenbaum #include "dt_data.h" -#include "helpers/s2l_util.h" + +#include +#include #include #include -#include #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& 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 ff6f7f174..3d774d65f 100644 --- 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 a263d06d6..c0b05934b 100644 --- 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 71477472d..aca65c883 100644 --- 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 1d5caea7d..329d98900 100644 --- 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 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 1b6ba974f..2952d6745 100644 --- 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 c7dd6a771..07d8010fd 100644 --- 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 78da99984..b06b6ced4 100644 --- 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 4fa669428..8fb343049 100644 --- 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 d74a3c70e..ad57b554a 100644 --- 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");