spoa-modsecurity icon indicating copy to clipboard operation
spoa-modsecurity copied to clipboard

tx.param returns wrong value

Open ariopangarih opened this issue 2 years ago • 2 comments

Hi! I'm using modsecurity v2.94 version and using your great code. But i meet problem when i try to make a simple modsec rules.

SecRule REQUEST_URI "@contains dangerous" "id:12345,phase:1,deny,setvar:TX.dangerous=1,status:403,msg:'URL contains dangerous'"

modsec blocks successfully by 403 but page still show 404 Apache (mean Haproxy didn't block the request).

Any help for this issue?

Btw thanks for sharing this code makes me love in networking tech!

ariopangarih avatar Jul 23 '23 22:07 ariopangarih

Hi there,

I personally started working with modsecurity (V2.9.7) and haproxy a week ago, so there may be some errors in my reasoning.

I had the same problem as you with another custom rule. Modsec was logging it as a deny, but the flag in my haproxy stayed at '-101' as if nothing was wrong... By looking into the origin of the problem, I've been able to trace it back to a logic error. It occurs in the modsec_wrapper.c code around line 600

	/* Process request headers analysis. */
	status = modsecProcessRequestHeaders(req);
	if (status != DECLINED && status != DONE)
		return_code = status;

	/* Process request body analysis. */
	status = modsecProcessRequestBody(req);
	if (status != DECLINED && status != DONE)
		return_code = status;

	/* End processing. */

	fail = 0;
	if (return_code == -1)
		return_code = 0;

The phase 1 keyword means it handle in the modsecProcessRequestHeaders(req) function but the spoa agent processes (and resets) the status variable with the line status=modsecProcessRequestBody(req) function. So i guess that's why haproxy reseve a "everthing is fine" info.

So the solution that i thought of were:

  • Put an if statement between the two functions.
  • Change my rule to phase 2, instead of phase 1.

In my case, i choose the second option because i didn't know the design logic in the first place.

Cpt-Graby avatar Nov 02 '23 12:11 Cpt-Graby

Same here. Note that value -101 corresponds to AP_NOBODY_READ which seems to be a variant of status code DECLINED (thus ModSecurity might in fact rather convert it to DECLINED).

Maybe it would be an option to include AP_NOBODY_READ in the conditions for both phases:

--- modsecurity-spoa.orig/modsec_wrapper.c  2025-03-21 10:37:56.195279627 +0100
+++ modsecurity-spoa.mod/modsec_wrapper.c   2025-03-21 10:38:21.067901236 +0100
@@ -602,12 +602,12 @@
 
 	/* Process request headers analysis. */
 	status = modsecProcessRequestHeaders(req);
-	if (status != DECLINED && status != DONE)
+	if (status != DECLINED && status != AP_NOBODY_READ && status != DONE)
 		return_code = status;
 
 	/* Process request body analysis. */
 	status = modsecProcessRequestBody(req);
-	if (status != DECLINED && status != DONE)
+	if (status != DECLINED && status != AP_NOBODY_READ && status != DONE)
 		return_code = status;
 
 	/* End processing. */

For sure somebody with deeper understanding of the implications this patch brings should have a look first.

buzz-tee avatar Mar 21 '25 09:03 buzz-tee