From: Oleksii Shumeiko -X (oshumeik - SOFTSERVE INC at Cisco) Date: Mon, 16 Jan 2023 08:13:37 +0000 (+0000) Subject: Pull request #3720: src: fix config parsing issues seen on 32bit systems X-Git-Tag: 3.1.52.0~3 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=d124f15df843e0df32dc6cd610a61a82c9611534;p=thirdparty%2Fsnort3.git Pull request #3720: src: fix config parsing issues seen on 32bit systems Merge in SNORT/snort3 from ~ASERBENI/snort3:32bit_issue to master Squashed commit of the following: commit 8137a4fca03573398a89f011ce3e66743b9c4154 Author: Andrii Serbeniuk Date: Wed Jan 4 17:15:04 2023 +0200 src: address numbers parsing related concerns commit 99895d8af9eb73b5646d54dc063322b910e467ea Author: Andrii Serbeniuk Date: Wed Jan 4 15:35:20 2023 +0200 framework: add strtoul methods to Value class commit 8e431851bc5416cb845684108d5a3c0a2407ecc3 Author: Andrii Serbeniuk Date: Wed Dec 21 16:48:01 2022 +0200 framework: change range check types to int64_t long may not be enough on 32bit platforms, where it's only 4 bytes long. issue initially found with seq ips option, where a valid value of 3,927,875,496 would be perceived as erroneous because it would not fit in 4 byte signed long (max value is 2,147,483,647) commit 59fec3d494cc8560754123688c4d9de7e216bbee Author: Andrii Serbeniuk Date: Tue Dec 20 14:38:30 2022 -0500 dce_rpc: add errno resets during uuid parsing --- diff --git a/src/framework/range.cc b/src/framework/range.cc index f0cb4e62b..effe5e418 100644 --- a/src/framework/range.cc +++ b/src/framework/range.cc @@ -145,7 +145,7 @@ static bool get_op(const string& s, RangeCheck::Op& op) return true; } -static bool get_num(const string& s, long& num) +static bool get_num(const string& s, int64_t& num) { if ( s.empty() ) { @@ -154,7 +154,7 @@ static bool get_num(const string& s, long& num) } errno = 0; char* end = nullptr; - num = strtol(s.c_str(), &end, 0); + num = strtoll(s.c_str(), &end, 0); return !errno and !*end; } @@ -229,7 +229,7 @@ bool RangeCheck::parse(const char* s) return true; } -bool RangeCheck::eval(long c) const +bool RangeCheck::eval(int64_t c) const { switch ( op ) { @@ -274,7 +274,7 @@ bool RangeCheck::validate(const char* s, const char* r) // require no leading or trailing whitespace // and either # | #: | :# | #:# // where # is a valid pos or neg dec, hex, or octal number - long v_min, v_max; + int64_t v_min, v_max; if ( op == LG or op == LEG ) { @@ -292,7 +292,7 @@ bool RangeCheck::validate(const char* s, const char* r) if ( *r != ':' ) { - long low = strtol(r, nullptr, 0); + int64_t low = strtoll(r, nullptr, 0); if ( v_min < low ) return false; @@ -302,7 +302,7 @@ bool RangeCheck::validate(const char* s, const char* r) if ( t && *++t ) { - long hi = strtol(t, nullptr, 0); + int64_t hi = strtoll(t, nullptr, 0); if ( v_max > hi ) return false; diff --git a/src/framework/range.h b/src/framework/range.h index ceafc5962..6c0816db1 100644 --- a/src/framework/range.h +++ b/src/framework/range.h @@ -44,8 +44,8 @@ public: }; Op op = MAX; - long min = 0; - long max = 0; + int64_t min = 0; + int64_t max = 0; bool operator==(const RangeCheck&) const; @@ -53,7 +53,7 @@ public: bool is_set() const; // FIXIT-L add ttl style syntax bool parse(const char* s); - bool eval(long) const; + bool eval(int64_t) const; bool validate(const char* s, const char* r); uint32_t hash() const; }; diff --git a/src/framework/value.cc b/src/framework/value.cc index 7174148c9..c909d8b04 100644 --- a/src/framework/value.cc +++ b/src/framework/value.cc @@ -186,6 +186,40 @@ bool Value::strtol(long& n, const std::string& tok) const return true; } +bool Value::strtoul(unsigned long& n) const +{ + const char* s = str.c_str(); + + if ( !*s ) + return false; + + char* end = nullptr; + + n = ::strtoul(s, &end, 0); + + if ( *end ) + return false; + + return true; +} + +bool Value::strtoul(unsigned long& n, const std::string& tok) const +{ + const char* s = tok.c_str(); + + if ( !*s ) + return false; + + char* end = nullptr; + + n = ::strtoul(s, &end, 0); + + if ( *end ) + return false; + + return true; +} + std::string Value::get_as_string() const { std::string value_str = str; diff --git a/src/framework/value.h b/src/framework/value.h index 39107803d..af5a43b66 100644 --- a/src/framework/value.h +++ b/src/framework/value.h @@ -176,6 +176,8 @@ public: bool strtol(long&) const; bool strtol(long&, const std::string&) const; + bool strtoul(unsigned long&) const; + bool strtoul(unsigned long&, const std::string&) const; bool operator==(const char* s) const { return str == s; } diff --git a/src/helpers/discovery_filter.cc b/src/helpers/discovery_filter.cc index c075a53a2..c490f33d4 100644 --- a/src/helpers/discovery_filter.cc +++ b/src/helpers/discovery_filter.cc @@ -94,7 +94,7 @@ DiscoveryFilter::DiscoveryFilter(const string& conf_path) intf = 0; else { - intf = strtol(config_intf.c_str(), nullptr, 0); + intf = strtoll(config_intf.c_str(), nullptr, 0); if ( intf < 1 or intf >= DF_ANY_INTF ) { WarningMessage("Discovery Filter: Invalid interface at line %u from %s;" diff --git a/src/ips_options/ips_byte_math.cc b/src/ips_options/ips_byte_math.cc index 8b4b0b347..2fa337299 100644 --- a/src/ips_options/ips_byte_math.cc +++ b/src/ips_options/ips_byte_math.cc @@ -354,8 +354,8 @@ bool ByteMathModule::set(const char*, Value& v, SnortConfig*) else if (v.is("rvalue")) { - long n; - if (v.strtol(n)) + unsigned long n; + if (v.strtoul(n)) { if (n == 0) return false; @@ -1486,6 +1486,16 @@ TEST_CASE("ByteMathModule::set valid", "[ips_byte_math]") CHECK(obj.set(nullptr, v, nullptr)); CHECK(strcmp(obj.off_var.c_str(), "off_test_var") == 0); } + SECTION("rvalue isn't truncated") + { + Value v("4294967295"); + Parameter p{"rvalue", Parameter::PT_STRING, nullptr, nullptr, + "value to use mathematical operation against"}; + v.set(&p); + + CHECK(obj.set(nullptr, v, nullptr)); + CHECK(obj.data.rvalue == 4294967295UL); + } delete[] obj.data.result_name; } diff --git a/src/ips_options/ips_byte_test.cc b/src/ips_options/ips_byte_test.cc index dc6bbf7dd..882e1e496 100644 --- a/src/ips_options/ips_byte_test.cc +++ b/src/ips_options/ips_byte_test.cc @@ -503,8 +503,8 @@ bool ByteTestModule::set(const char*, Value& v, SnortConfig*) else if (v.is("~compare")) { - long n; - if (v.strtol(n)) + unsigned long n; + if (v.strtoul(n)) data.cmp_value = n; else cmp_var = v.get_string(); @@ -1092,6 +1092,16 @@ TEST_CASE("ByteTestModule test", "[ips_byte_test]") value_tmp.set(¶m); REQUIRE(module_test.set(nullptr, value_tmp, nullptr) == true); } + + SECTION("Value isn't truncated") + { + Value value_tmp("4294967295"); + Parameter param("~compare", snort::Parameter::Type::PT_BOOL, + nullptr, "default", "help"); + value_tmp.set(¶m); + REQUIRE(module_test.set(nullptr, value_tmp, nullptr) == true); + REQUIRE(module_test.data.cmp_value == 4294967295UL); + } } SECTION("Case param \"~offset\"") diff --git a/src/ips_options/ips_isdataat.cc b/src/ips_options/ips_isdataat.cc index c7851ad04..cf9813be2 100644 --- a/src/ips_options/ips_isdataat.cc +++ b/src/ips_options/ips_isdataat.cc @@ -188,7 +188,7 @@ static void isdataat_parse(const char* data, IsDataAtData* idx) char* endp; idx->offset_var = IPS_OPTIONS_NO_VAR; - idx->offset = strtol(offset, &endp, 10); + idx->offset = strtoul(offset, &endp, 10); if (offset == endp) { diff --git a/src/ips_options/ips_rpc.cc b/src/ips_options/ips_rpc.cc index a35f76db6..3ea120bca 100644 --- a/src/ips_options/ips_rpc.cc +++ b/src/ips_options/ips_rpc.cc @@ -285,7 +285,7 @@ bool RpcModule::set(const Value& v, uint32_t& field, int flag) errno = 0; char* end = nullptr; - int64_t num = (int64_t)strtol(v.get_string(), &end, 0); + int64_t num = strtoll(v.get_string(), &end, 0); if ( *end or errno or num < 0 or num > 0xFFFFFFFF ) return false; diff --git a/src/managers/ips_manager.cc b/src/managers/ips_manager.cc index 34d9954b4..d89723233 100644 --- a/src/managers/ips_manager.cc +++ b/src/managers/ips_manager.cc @@ -130,10 +130,10 @@ static bool set_arg( if ( p->is_wild_card() ) val = opt; - long n = (long)strtoll(val, &end, 0); + int64_t n = strtoll(val, &end, 0); if ( !*end ) - v.set(n); + v.set((double)n); else ok = false; } diff --git a/src/network_inspectors/appid/app_info_table.cc b/src/network_inspectors/appid/app_info_table.cc index 3f66b37c8..27f9af1c5 100644 --- a/src/network_inspectors/appid/app_info_table.cc +++ b/src/network_inspectors/appid/app_info_table.cc @@ -690,7 +690,7 @@ void AppInfoManager::init_appid_info_table(const AppIdConfig& config, snort_free(app_name); continue; } - service_id = strtol(token, nullptr, 10); + service_id = strtoul(token, nullptr, 10); token = strtok_r(nullptr, CONF_SEPARATORS, &context); if (!token) @@ -699,7 +699,7 @@ void AppInfoManager::init_appid_info_table(const AppIdConfig& config, snort_free(app_name); continue; } - client_id = strtol(token, nullptr, 10); + client_id = strtoul(token, nullptr, 10); token = strtok_r(nullptr, CONF_SEPARATORS, &context); if (!token) @@ -708,7 +708,7 @@ void AppInfoManager::init_appid_info_table(const AppIdConfig& config, snort_free(app_name); continue; } - payload_id = strtol(token, nullptr, 10); + payload_id = strtoul(token, nullptr, 10); AppInfoTableEntry* entry = new AppInfoTableEntry(app_id, app_name, service_id, client_id, payload_id); diff --git a/src/network_inspectors/reputation/reputation_parse.cc b/src/network_inspectors/reputation/reputation_parse.cc index d6c88f267..2cb632a1c 100644 --- a/src/network_inspectors/reputation/reputation_parse.cc +++ b/src/network_inspectors/reputation/reputation_parse.cc @@ -427,6 +427,7 @@ static int snort_pton(char const* src, SfCidr* dest) if ( *cidrbuf ) { char* end; + errno = 0; int value = strtol(cidrbuf, &end, 10); if ( value > dest->get_bits() || value <= 0 || errno == ERANGE ) diff --git a/src/service_inspectors/dce_rpc/ips_dce_iface.cc b/src/service_inspectors/dce_rpc/ips_dce_iface.cc index 47e25fcca..ed20ffff4 100644 --- a/src/service_inspectors/dce_rpc/ips_dce_iface.cc +++ b/src/service_inspectors/dce_rpc/ips_dce_iface.cc @@ -93,6 +93,7 @@ static bool DCE2_ParseIface(char* token, Uuid* uuid) if (strlen(if_hex) != DCE2_IFACE__TIME_LOW_LEN) return false; + errno = 0; time_low = strtoul(if_hex, &endptr, 16); if ((errno == ERANGE) || (*endptr != '\0')) return false; @@ -109,6 +110,7 @@ static bool DCE2_ParseIface(char* token, Uuid* uuid) if (strlen(if_hex) != DCE2_IFACE__TIME_MID_LEN) return false; + errno = 0; time_mid = strtoul(if_hex, &endptr, 16); if ((errno == ERANGE) || (*endptr != '\0')) return false; @@ -126,6 +128,7 @@ static bool DCE2_ParseIface(char* token, Uuid* uuid) if (strlen(if_hex) != DCE2_IFACE__TIME_HIGH_LEN) return false; + errno = 0; time_high = strtoul(if_hex, &endptr, 16); if ((errno == ERANGE) || (*endptr != '\0')) return false; @@ -144,6 +147,7 @@ static bool DCE2_ParseIface(char* token, Uuid* uuid) return false; /* Work backwards */ + errno = 0; clock_seq_low = strtoul(&if_hex[2], &endptr, 16); if ((errno == ERANGE) || (*endptr != '\0')) return false; @@ -153,6 +157,7 @@ static bool DCE2_ParseIface(char* token, Uuid* uuid) /* Set third byte to null so we can _dpd.SnortStrtoul the first part */ if_hex[2] = '\x00'; + errno = 0; clock_seq_and_reserved = strtoul(if_hex, &endptr, 16); if ((errno == ERANGE) || (*endptr != '\0')) return false; @@ -174,6 +179,7 @@ static bool DCE2_ParseIface(char* token, Uuid* uuid) (i >= 0) && (j >= 0); i -= 2, j--) { + errno = 0; /* Only giving strtoul 1 byte */ uuid->node[j] = (uint8_t)strtoul(&if_hex[i], &endptr, 16); if ((errno == ERANGE) || (*endptr != '\0'))