Update SQLi/XSS operators for libinjection v4.0.0 cleaned by Easton97-Jens · Pull Request #3528 · owasp-modsecurity/ModSecurity

Easton97-Jens · 2026-03-29T09:46:29Z

what

Updated the SQLi and XSS detection operators to support the newer libinjection return codes (injection_result_t).
Added explicit handling for TRUE, FALSE, and ERROR results from libinjection_sqli and libinjection_xss.
Treated LIBINJECTION_RESULT_ERROR as a fail-safe match to avoid missing potentially malicious input.
Preserved diagnostic context by storing raw input in TX.0 when capture is enabled, even on parser errors.
Maintained previous behavior for positive detections (fingerprints for SQLi, input storage for XSS).
Added regression tests for benign inputs to ensure no false positives are introduced.

why

Newer versions of libinjection introduced injection_result_t, requiring explicit handling in ModSecurity operators.
Without this update, parser errors could be handled inconsistently and weaken fail-safe detection behavior.
Treating parser errors as matches ensures conservative and security-focused handling when input cannot be reliably parsed.
Preserving captured input during error cases improves debugging and visibility for rule authors.
Regression tests ensure the updated logic does not introduce false positives for safe inputs.

references

Related to adapting ModSecurity to newer libinjection APIs.
Adds regression coverage for parser error handling and safe input validation.

Copilot

Pull request overview

Updates ModSecurity’s @detectSQLi / @detectXSS operators to support libinjection v4’s injection_result_t return codes, including explicit fail-safe handling for parser errors, and expands regression coverage around detection/false-positive behavior.

Changes:

Add shared helpers for interpreting libinjection TRUE/FALSE/ERROR results.
Update DetectSQLi / DetectXSS to treat LIBINJECTION_RESULT_ERROR as a match and preserve capture behavior.
Expand regression test cases for multiple XSS/SQLi payloads and benign inputs.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 10 comments.

Show a summary per file

File	Description
`src/operators/libinjection_utils.h`	Adds shared helpers to map libinjection results to match/no-match semantics and diagnostic strings.
`src/operators/detect_xss.cc`	Switches XSS operator logic to `injection_result_t` and adds explicit handling for `TRUE/FALSE/ERROR`.
`src/operators/detect_sqli.cc`	Switches SQLi operator logic to `injection_result_t`, modernizes fingerprint storage, and handles `TRUE/FALSE/ERROR`.
`test/test-cases/regression/operator-detectxss.json`	Adds multiple positive and negative XSS regression cases.
`test/test-cases/regression/operator-detectsqli.json`	Adds multiple positive and negative SQLi regression cases (including fingerprint expectations).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

src/operators/detect_xss.cc

src/operators/detect_sqli.cc

src/operators/detect_xss.cc

src/operators/detect_sqli.cc

…for-modsecurity-operators Add regression coverage for detectSQLi/detectXSS capture semantics

…for-libinjection-result_error Add libinjection adapter for test overrides and improve libinjection error handling

Copilot

Pull request overview

Copilot reviewed 16 out of 16 changed files in this pull request and generated 1 comment.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-01T12:32:49Z

test/unit/unit.cc

+        if (t.capture) {
+            auto tx0 = transaction.m_collections.m_tx_collection->resolveFirst("0");
+            if (tx0 != nullptr) {
+                result.output = *tx0;
+            }


OperatorTest::eval reads TX.0 from the shared Transaction when capture is enabled. In mtstress mode the same transaction instance is shared across many threads, so concurrent writes/reads of the TX collection can cause data races and flaky/crashing unit tests (especially now that output comparisons are enforced). Consider using a per-thread Transaction in mtstress runs (or disabling capture/output assertions under mtstress) so TX.0 access is thread-safe/deterministic.

@Easton97-Jens,

even thought this case (shared transaction objects) is a bit far from the reality, I think it would be nice to find a solution to meet this expectation.

(Moreover, there is no mtstress option at all between test cases.)

Could you take a look at this?

If you think it's not necessarily now, we can skip now, and we might add this later (with new test cases...)

…n-v4.0-final

…transaction-usage Use ModSecurityTestContext to create per-thread transactions in multithreaded tests

Copilot

Pull request overview

Copilot reviewed 17 out of 17 changed files in this pull request and generated 6 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

src/operators/libinjection_adapter.h

src/operators/libinjection_adapter.cc

Copilot · 2026-04-01T20:09:11Z

src/operators/libinjection_adapter.h

+
+void setLibinjectionSQLiOverrideForTesting(DetectSQLiFn fn);
+void setLibinjectionXSSOverrideForTesting(DetectXSSFn fn);
+void clearLibinjectionOverridesForTesting();


The functions setLibinjection*OverrideForTesting()/clearLibinjectionOverridesForTesting() are compiled into the production library and become part of the exported symbol surface, despite being test-only hooks. Consider compiling these APIs only for test builds (e.g., behind a dedicated macro) or otherwise hiding them, so production consumers can’t accidentally (or intentionally) alter libinjection behavior at runtime.

Suggested change

void setLibinjectionSQLiOverrideForTesting(DetectSQLiFn fn);

void setLibinjectionXSSOverrideForTesting(DetectXSSFn fn);

void clearLibinjectionOverridesForTesting();

// Test-only hooks to override libinjection behavior.

// These should be enabled only in test builds by defining

// MODSECURITY_ENABLE_LIBINJECTION_TESTING_HOOKS.

#ifdef MODSECURITY_ENABLE_LIBINJECTION_TESTING_HOOKS

void setLibinjectionSQLiOverrideForTesting(DetectSQLiFn fn);

void setLibinjectionXSSOverrideForTesting(DetectXSSFn fn);

void clearLibinjectionOverridesForTesting();

#endif // MODSECURITY_ENABLE_LIBINJECTION_TESTING_HOOKS

Copilot · 2026-04-01T20:09:12Z

test/unit/unit.cc

-            [&item, &t, &result, &transaction]()
+            [&item, &t, &result, &context]()
            {
+                auto transaction = context.create_transaction();


In mtstress mode each thread now creates its own Transaction, but they still share the same ModSecurityTestContext instance. ModSecurityTestContext::create_transaction() passes a pointer to the shared m_server_log (std::stringstream) into each Transaction, and the log callback appends to it; this is not thread-safe and can still cause data races/flaky mtstress runs. Consider giving each thread its own log buffer (or disabling server-log capture during mtstress) to make the stress test deterministic and race-free.

Suggested change

auto transaction = context.create_transaction();

// Use a per-thread copy of the test context so that each thread

// has its own server log buffer, avoiding data races on m_server_log.

auto localContext = context;

auto transaction = localContext.create_transaction();

Copilot · 2026-04-01T20:09:12Z

src/operators/detect_xss.cc

+            ms_dbg_a(t, 4,
+                std::string("libinjection parser error during XSS analysis (")
+                + libinjectionResultToString(xss_result)
+                + "); treating as match (fail-safe). Input: "
+                + input);


The new LIBINJECTION_RESULT_ERROR debug message logs the full raw input at debug level 4. To reduce log-forging risk and avoid extremely large/binary log lines, please consider sanitizing/truncating the logged input (e.g., limit length and/or use the existing toHexIfNeeded/limitTo helpers used elsewhere for logging untrusted data).

Copilot · 2026-04-01T20:09:12Z

src/operators/detect_sqli.cc

+                std::string("libinjection parser error during SQLi analysis (")
+                + libinjectionResultToString(sqli_result)
+                + "); treating as match (fail-safe). Input: '"
+                + input + "'");


The new LIBINJECTION_RESULT_ERROR debug message includes the full raw input at debug level 4. Consider limiting/sanitizing what is logged (length cap and/or hex-escaping) to avoid log injection and excessive log volume on crafted inputs; other parts of the codebase use limitTo()/toHexIfNeeded() for this purpose.

Suggested change

+ input + "'");

+ limitTo(toHexIfNeeded(input), 1000) + "'");

sonarqubecloud · 2026-04-01T20:42:37Z

Quality Gate passed

Issues
0 New issues
10 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

jens added 3 commits March 29, 2026 11:31

Update libinjection to v4.0.0

58fc3da

Make detect_sqli and detect_xss compatible with libinjection v4

e1a527e

Update regression tests for libinjection v4 compatibility

d2cef41

airween requested a review from Copilot March 30, 2026 06:59

Copilot started reviewing on behalf of airween March 30, 2026 06:59 View session

Copilot AI reviewed Mar 30, 2026

View reviewed changes

jens and others added 3 commits March 30, 2026 10:10

syntax adjustment

9e66822

Add capture/non-capture regression coverage for detectSQLi/XSS

633f2eb

Merge pull request #27 from Easton97-Jens/codex/add-regression-tests-…

b264dbf

…for-modsecurity-operators Add regression coverage for detectSQLi/detectXSS capture semantics

Easton97-Jens requested a review from airween March 30, 2026 15:24

Easton97-Jens added 3 commits March 31, 2026 20:22

Fix Windows test include path and case-insensitive override matching

7cd1d67

Merge pull request #31 from Easton97-Jens/codex/add-regression-tests-…

2aed15b

…for-libinjection-result_error Add libinjection adapter for test overrides and improve libinjection error handling

Update libinjection_adapter.cc

0af7e13

airween requested a review from Copilot April 1, 2026 12:23

Copilot started reviewing on behalf of airween April 1, 2026 12:24 View session

Copilot AI reviewed Apr 1, 2026

View reviewed changes

Easton97-Jens added 4 commits April 1, 2026 15:55

Merge branch 'owasp-modsecurity:v3/master' into v3/master-libinjectio…

d8c7395

…n-v4.0-final

Add libinjection_error.h to Makefile.am

d6648d1

Isolate transaction state in multithreaded unit tests

19ea6d0

Merge pull request #32 from Easton97-Jens/codex/review-multithreaded-…

e152a09

…transaction-usage Use ModSecurityTestContext to create per-thread transactions in multithreaded tests

airween requested a review from Copilot April 1, 2026 20:02

Copilot started reviewing on behalf of airween April 1, 2026 20:02 View session

Copilot AI reviewed Apr 1, 2026

View reviewed changes

Easton97-Jens added 3 commits April 1, 2026 22:09

Merge branch 'v3/master' into v3/master-libinjection-v4.0-final

719d172

Update libinjection_adapter.cc

468f681

Update libinjection_adapter.h

d19f58b

-                auto transaction = context.create_transaction();
+                // Use a per-thread copy of the test context so that each thread
+                // has its own server log buffer, avoiding data races on m_server_log.
+                auto localContext = context;
+                auto transaction = localContext.create_transaction();

	+ input + "'");
	+ limitTo(toHexIfNeeded(input), 1000) + "'");

Conversation

Easton97-Jens commented Mar 29, 2026

what

why

references

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Apr 1, 2026

Choose a reason for hiding this comment

Uh oh!

airween Apr 1, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Copilot AI Apr 1, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 1, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 1, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 1, 2026

Choose a reason for hiding this comment

Uh oh!

sonarqubecloud bot commented Apr 1, 2026

Quality Gate passed

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants