Merge pull request #3421 from airween/v3/secreqbodylimit

fix: integer type limits, 2nd try

headers/modsecurity/rules_set_properties.h

@@ -13,6 +13,14 @@
  *
  */
 
+#ifdef WIN32
+#ifdef max
+#undef max
+#endif
+#ifdef min
+#undef min
+#endif
+#endif
 
 #ifdef __cplusplus
 #include <ctime>
@@ -22,6 +30,8 @@
 #include <list>
 #include <set>
 #include <cstring>
+#include <limits>
+#include <cstdint>
 #endif
 
 
@@ -68,46 +78,132 @@ class Driver;
 using modsecurity::debug_log::DebugLog;
 using modsecurity::audit_log::AuditLog;
 
-/** @ingroup ModSecurity_CPP_API */
-class ConfigInt {
- public:
-    ConfigInt() : m_set(false), m_value(0) { }
-    bool m_set;
-    int m_value;
+// template for different numeric int types
+template <typename T>
+class ConfigValue {
+public:
+    bool m_set = false;
+    T m_value = 0;
 
-    void merge(const ConfigInt *from) {
-        if (m_set == true || from->m_set == false) {
+    ConfigValue() = default;
+
+    void merge(const ConfigValue<T>* from) {
+        if (m_set || !from->m_set) {
             return;
         }
         m_set = true;
         m_value = from->m_value;
-        return;
+    }
+
+    // default parser
+    bool parse(const std::string& a, std::string* errmsg = nullptr) {
+
+        // use an alias type because the template can convert both signed and unsigned int
+        using LimitSigned = std::conditional_t<std::is_signed_v<T>, std::int64_t, std::uint64_t>;
+        LimitSigned val;
+
+        // clear errno variable, wee need that later
+        errno = 0;
+
+        try {
+            if constexpr (std::is_signed_v<T>) {
+                val = static_cast<std::int64_t>(std::stoll(a));
+            } else {
+                val = static_cast<std::uint64_t>(std::stoull(a));
+            }
+        }
+        catch (const std::invalid_argument&) {
+            // probably can't occur, but we handle it anyway
+            set_error(errmsg, "Invalid number format (not numeric)");
+            return false;
+        }
+        catch (const std::out_of_range&) {
+            // the value is out of range, we can not handle it
+            set_error(errmsg, "Number out of range");
+            return false;
+        }
+        catch (...) { // NOSONAR
+            // we don't need to handle all exceptions, the engine's BISON parser
+            // does not allow other symbols than numbers
+            set_error(errmsg, "An unknown error occurred while parsing number.");
+            return false;
+        }
+
+        if (
+            // The first condition will be true when the value is bigger than int64/uint64 maximum value.
+            // The second condition checks whether the value fits into int64/uint64, but not
+            // into the designed type, e.g., uint32; in that case the errno will be 0, but
+            // we must check the value is not bigger than the defined maximum of the class.
+            (errno == ERANGE && val == std::numeric_limits<LimitSigned>::max())
+            ||
+            (val > static_cast<LimitSigned>(maxValue()))
+        ) {
+            set_error(errmsg, "Value is too big.");
+            return false;
+        }
+
+        if (
+            // same as above
+            (errno == ERANGE && val == std::numeric_limits<LimitSigned>::min())
+            ||
+            (val < static_cast<LimitSigned>(minValue()))
+        ) {
+            set_error(errmsg, "Value is too small.");
+            return false;
+        }
+
+        m_value = static_cast<T>(val);
+        m_set = true;
+        return true;
+
+    }
+
+protected:
+    // derived classes must implement the maxValue
+    virtual T maxValue() const = 0;
+    // minValue is optional
+    virtual T minValue() const { return 0; }
+
+private:
+    static inline void set_error(std::string* err, const char* msg) {
+        if (err) {
+            *err = msg;
+        }
     }
 };
 
+/** @ingroup ModSecurity_CPP_API */
 
-class ConfigDouble {
- public:
-    ConfigDouble() : m_set(false), m_value(0) { }
-    bool m_set;
-    double m_value;
+class ConfigInt : public ConfigValue<int32_t> {
+protected:
+    int32_t minValue() const override {
+        return std::numeric_limits<int32_t>::min();
+    }
+    int32_t maxValue() const override {
+        return std::numeric_limits<int32_t>::max();
+    }
+};
 
-    void merge(const ConfigDouble *from) {
-        if (m_set == true || from->m_set == false) {
-            return;
-        }
-        m_set = true;
-        m_value = from->m_value;
-        return;
+class ConfigUnsignedInt : public ConfigValue<uint32_t> {
+protected:
+    uint32_t maxValue() const override {
+        return std::numeric_limits<uint32_t>::max();
+    }
+};
+
+class ConfigUnsignedLong : public ConfigValue<uint64_t> {
+protected:
+    uint64_t maxValue() const override {
+        return std::numeric_limits<uint64_t>::max();
     }
 };
 
 
 class ConfigString {
  public:
-    ConfigString() : m_set(false), m_value("") { }
-    bool m_set;
-    std::string m_value;
+    bool m_set = false;
+    std::string m_value = "";
+    ConfigString() = default;
 
     void merge(const ConfigString *from) {
         if (m_set == true || from->m_set == false) {
@@ -122,10 +218,10 @@ class ConfigString {
 
 class ConfigSet {
  public:
-    ConfigSet() : m_set(false), m_clear(false) { }
-    bool m_set;
-    bool m_clear;
+    bool m_set = false;
+    bool m_clear = false;
     std::set<std::string> m_value;
+    ConfigSet() = default;
 };
 
 
@@ -504,14 +600,14 @@ class RulesSetProperties {
     ConfigXMLParseXmlIntoArgs m_secXMLParseXmlIntoArgs;
     ConfigBoolean m_tmpSaveUploadedFiles;
     ConfigBoolean m_uploadKeepFiles;
-    ConfigDouble m_argumentsLimit;
-    ConfigDouble m_requestBodyJsonDepthLimit;
-    ConfigDouble m_requestBodyLimit;
-    ConfigDouble m_requestBodyNoFilesLimit;
-    ConfigDouble m_responseBodyLimit;
-    ConfigInt m_pcreMatchLimit;
-    ConfigInt m_uploadFileLimit;
-    ConfigInt m_uploadFileMode;
+    ConfigUnsignedInt m_argumentsLimit;
+    ConfigUnsignedInt m_requestBodyJsonDepthLimit;
+    ConfigUnsignedLong m_requestBodyLimit;
+    ConfigUnsignedLong m_requestBodyNoFilesLimit;
+    ConfigUnsignedLong m_responseBodyLimit;
+    ConfigUnsignedInt m_pcreMatchLimit;
+    ConfigUnsignedInt m_uploadFileLimit;
+    ConfigUnsignedInt m_uploadFileMode;
     DebugLog *m_debugLog;
     OnFailedRemoteRulesAction m_remoteRulesActionOnFailed;
     RuleEngine m_secRuleEngine;

src/parser/seclang-parser.yy

@@ -831,13 +831,19 @@ audit_log:
       }
     | CONFIG_UPLOAD_FILE_LIMIT
       {
-        driver.m_uploadFileLimit.m_set = true;
-        driver.m_uploadFileLimit.m_value = strtol($1.c_str(), NULL, 10);
+        std::string errmsg = "";
+        if (driver.m_uploadFileLimit.parse(std::string($1), &errmsg) != true) {
+          driver.error(@0, "Failed to parse SecUploadFileLimit: " + errmsg);
+          YYERROR;
+        }
       }
     | CONFIG_UPLOAD_FILE_MODE
       {
-        driver.m_uploadFileMode.m_set = true;
-        driver.m_uploadFileMode.m_value = strtol($1.c_str(), NULL, 8);
+        std::string errmsg = "";
+        if (driver.m_uploadFileMode.parse(std::string($1), &errmsg) != true) {
+          driver.error(@0, "Failed to parse SecUploadFileMode: " + errmsg);
+          YYERROR;
+        }
       }
     | CONFIG_UPLOAD_DIR
       {
@@ -1608,13 +1614,19 @@ expression:
     /* Body limits */
     | CONFIG_DIR_REQ_BODY_LIMIT
       {
-        driver.m_requestBodyLimit.m_set = true;
-        driver.m_requestBodyLimit.m_value = atoi($1.c_str());
+        std::string errmsg = "";
+        if (driver.m_requestBodyLimit.parse(std::string($1), &errmsg) != true) {
+          driver.error(@0, "Failed to parse SecRequestBodyLimit: " + errmsg);
+          YYERROR;
+        }
       }
     | CONFIG_DIR_REQ_BODY_NO_FILES_LIMIT
       {
-        driver.m_requestBodyNoFilesLimit.m_set = true;
-        driver.m_requestBodyNoFilesLimit.m_value = atoi($1.c_str());
+        std::string errmsg = "";
+        if (driver.m_requestBodyNoFilesLimit.parse(std::string($1), &errmsg) != true) {
+          driver.error(@0, "Failed to parse SecRequestsBodyNoFilesLimit: " + errmsg);
+          YYERROR;
+        }
       }
     | CONFIG_DIR_REQ_BODY_IN_MEMORY_LIMIT
       {
@@ -1627,8 +1639,11 @@ expression:
       }
     | CONFIG_DIR_RES_BODY_LIMIT
       {
-        driver.m_responseBodyLimit.m_set = true;
-        driver.m_responseBodyLimit.m_value = atoi($1.c_str());
+        std::string errmsg = "";
+        if (driver.m_responseBodyLimit.parse(std::string($1), &errmsg) != true) {
+          driver.error(@0, "Failed to parse SecResponseBodyLimit: " + errmsg);
+          YYERROR;
+        }
       }
     | CONFIG_DIR_REQ_BODY_LIMIT_ACTION CONFIG_VALUE_PROCESS_PARTIAL
       {
@@ -1661,8 +1676,11 @@ expression:
 */
     | CONFIG_DIR_PCRE_MATCH_LIMIT
       {
-        driver.m_pcreMatchLimit.m_set = true;
-        driver.m_pcreMatchLimit.m_value = atoi($1.c_str());
+        std::string errmsg = "";
+        if (driver.m_pcreMatchLimit.parse(std::string($1), &errmsg) != true) {
+          driver.error(@0, "Failed to parse SecPcreMatchLimit: " + errmsg);
+          YYERROR;
+        }
       }
     | CONGIG_DIR_RESPONSE_BODY_MP
       {