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 <aserbeni@cisco.com>
Date:   Wed Jan 4 17:15:04 2023 +0200

    src: address numbers parsing related concerns

commit 99895d8af9eb73b5646d54dc063322b910e467ea
Author: Andrii Serbeniuk <aserbeni@cisco.com>
Date:   Wed Jan 4 15:35:20 2023 +0200

    framework: add strtoul methods to Value class

commit 8e431851bc5416cb845684108d5a3c0a2407ecc3
Author: Andrii Serbeniuk <aserbeni@cisco.com>
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 <aserbeni@cisco.com>
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 f0cb4e62b3721e93a5ba829e4954e21753338134..effe5e418035a7f09f072dad92210c78f6de7662 100644 (file)
--- 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 ceafc5962af91c442359a04aac0619288880cac8..6c0816db1ee20c236f0b45321d15dbd73411a2e6 100644 (file)
--- 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 7174148c9118062c762d2e0585fb392a8e6b773f..c909d8b0405f44fbcfb89b3efad58338631182d6 100644 (file)
--- 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 39107803dce41f2a8f0f5060a63e2a1c65906a7c..af5a43b6674927f13cfd9548ff0257e9c8267512 100644 (file)
--- 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 c075a53a207ffdaf8c7a6e1c84b1c76e57640d99..c490f33d45434ae524e80dc51c6c5098cef4e722 100644 (file)
--- 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 8b4b0b347d44035301c71186d927e7777f30abaa..2fa3372999305ab25035659bf05563ded47307e9 100644 (file)
--- 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 dc6bbf7dddf8a8e26ae3bf6e195e2f091b5e462d..882e1e496bc228348a6fb84df617bb6f72a01316 100644 (file)
--- 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(&param);
                 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(&param);
+                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 c7851ad04dbc5bfc732eb5d0e7208a799f909321..cf9813be2f8e0ee585a69abdff6d9dd4c2fd976a 100644 (file)
--- 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 a35f76db6d3e0a474a5aa63b936a1d0ff6852869..3ea120bca628a2577b21b69a5a29c243233a3b98 100644 (file)
--- 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 34d9954b4b29cf6c71f142ee9f20e326e32bd642..d89723233d7c2f142fcba543ef0470af62dfb6ab 100644 (file)
--- 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 3f66b37c84db688c3845559614848f80dbeec2e0..27f9af1c5d9af21055052104b254024f58ac7569 100644 (file)
--- 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 d6c88f26751b63864bc5297f3392ed4fdda39d11..2cb632a1c58ac751374f5984361a079545528ba3 100644 (file)
--- 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 47e25fcca86d5dc7e9be9baa549b2f54cbf2da21..ed20ffff429796a64fc6a87ec03bea42ee18dc55 100644 (file)
--- 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'))