Skip to content

Commit 8ed97d6

Browse files
airweenairween
committed
fix: recovery context after internal redirect, re-add owasp-modsecurity#273 
1 parent 0b4f0cf commit 8ed97d6

10 files changed

+219
-26
lines changed

‎.github/nginx/docs/403.html‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
<html>
2+
<head>
3+
<title>403</title>
4+
</head>
5+
6+
<body>
7+
Forbidden 403 - custom error page.
8+
</body>
9+
</html>
10+

‎.github/nginx/nginx.conf.redir‎

Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,90 @@
1+
user www-data;
2+
worker_processes auto;
3+
pid /run/nginx.pid;
4+
worker_cpu_affinity auto;
5+
6+
#working_directory /tmp/cores/;
7+
worker_rlimit_core 2000M;
8+
debug_points abort;
9+
10+
#load_module /usr/local/nginx/modules/ngx_http_modsecurity_module.so;
11+
12+
events {
13+
worker_connections 768;
14+
# multi_accept on;
15+
# use epoll;
16+
}
17+
18+
worker_rlimit_nofile 33268;
19+
20+
#daemon off;
21+
#master_process off;
22+
23+
http {
24+
25+
##
26+
# Basic Settings
27+
##
28+
29+
types_hash_max_size 2048;
30+
31+
server_names_hash_bucket_size 64;
32+
33+
include mime.types;
34+
default_type application/octet-stream;
35+
36+
##
37+
# Logging Settings
38+
##
39+
40+
#access_log /dev/stdout;
41+
#error_log /dev/stdout info;
42+
access_log /usr/local/nginx/logs/access.log;
43+
error_log /usr/local/nginx/logs/error.log info;
44+
45+
server_tokens off;
46+
47+
proxy_hide_header X-Powered-By;
48+
49+
modsecurity on;
50+
51+
server {
52+
listen 80;
53+
server_name modsectest1;
54+
55+
modsecurity on;
56+
modsecurity_rules_file /home/runner/work/ModSecurity-nginx/ModSecurity-nginx/ModSecurity-nginx/.github/nginx/modsecurity.conf;
57+
root /usr/local/nginx/html/;
58+
59+
error_page 403 /403.html;
60+
61+
location /403.html {
62+
internal;
63+
}
64+
65+
location / {
66+
try_files $uri /index.html;
67+
}
68+
}
69+
70+
server {
71+
listen 80;
72+
server_name modsectest2;
73+
74+
modsecurity on;
75+
modsecurity_rules_file /home/runner/work/ModSecurity-nginx/ModSecurity-nginx/ModSecurity-nginx/.github/nginx/modsecurity.conf;
76+
root /usr/local/nginx/html/;
77+
78+
error_page 403 /403.html;
79+
80+
location /403.html {
81+
internal;
82+
}
83+
84+
location / {
85+
try_files $uri /index.html;
86+
}
87+
}
88+
89+
}
90+

‎.github/workflows/test.yml‎

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -140,6 +140,78 @@ jobs:
140140
echo "FAIL"
141141
exit 1
142142
fi
143+
- name: Start Nginx with redir
144+
run: |
145+
sudo killall nginx
146+
sudo /usr/local/nginx/sbin/nginx -c /home/runner/work/ModSecurity-nginx/ModSecurity-nginx/ModSecurity-nginx/.github/nginx/nginx.conf.redir
147+
- name: Run attack test vhost 1
148+
run: |
149+
status=$(curl -sSo /dev/null -w %{http_code} -I -X GET -H "Host: modsectest1" "http://localhost/?q=attack")
150+
if [ "${status}" == "403" ]; then
151+
echo "OK"
152+
else
153+
echo "FAIL"
154+
exit 1
155+
fi
156+
- name: Run non-attack test vhost 1 (redir config)
157+
run: |
158+
status=$(curl -sSo /dev/null -w %{http_code} -I -X GET -H "Host: modsectest1" "http://localhost/?q=1")
159+
if [ "${status}" == "200" ]; then
160+
echo "OK"
161+
else
162+
echo "FAIL"
163+
exit 1
164+
fi
165+
- name: Run attack test vhost 2 (redir config)
166+
run: |
167+
status=$(curl -sSo /dev/null -w %{http_code} -I -X GET -H "Host: modsectest2" "http://localhost/?q=attack")
168+
if [ "${status}" == "403" ]; then
169+
echo "OK"
170+
else
171+
echo "FAIL"
172+
exit 1
173+
fi
174+
- name: Run non-attack test vhost 2 (redir config)
175+
run: |
176+
status=$(curl -sSo /dev/null -w %{http_code} -I -X GET -H "Host: modsectest2" "http://localhost/?q=1")
177+
if [ "${status}" == "200" ]; then
178+
echo "OK"
179+
else
180+
echo "FAIL"
181+
exit 1
182+
fi
183+
- name: Run file consistency check 1 (redir config)
184+
run: |
185+
curl -sS "http://localhost/data50k.json" --output data50k.json
186+
if [ -f data50k.json ]; then
187+
diff data50k.json /usr/local/nginx/html/data50k.json > /dev/null
188+
if [ $? -eq 0 ]; then
189+
ls -l data50k.json /usr/local/nginx/html/data50k.json
190+
echo "OK"
191+
else
192+
echo "FAIL"
193+
exit 2
194+
fi
195+
else
196+
echo "FAIL"
197+
exit 1
198+
fi
199+
- name: Run file consistency check 2 (redir config)
200+
run: |
201+
curl -sS "http://localhost/plugged.png" --output plugged.png
202+
if [ -f plugged.png ]; then
203+
diff plugged.png /usr/local/nginx/html/plugged.png > /dev/null
204+
if [ $? -eq 0 ]; then
205+
ls -l plugged.png /usr/local/nginx/html/plugged.png
206+
echo "OK"
207+
else
208+
echo "FAIL"
209+
exit 2
210+
fi
211+
else
212+
echo "FAIL"
213+
exit 1
214+
fi
143215
144216
145217
build-windows:

‎src/ngx_http_modsecurity_body_filter.c‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ ngx_http_modsecurity_body_filter(ngx_http_request_t *r, ngx_chain_t *in)
5050
return ngx_http_next_body_filter(r, in);
5151
}
5252

53-
ctx = ngx_http_get_module_ctx(r, ngx_http_modsecurity_module);
53+
ctx = ngx_http_modsecurity_get_module_ctx(r);
5454

5555
dd("body filter, recovering ctx: %p", ctx);
5656

‎src/ngx_http_modsecurity_common.h‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,7 @@ typedef struct {
9999
unsigned processed:1;
100100
unsigned logged:1;
101101
unsigned intervention_triggered:1;
102+
unsigned request_body_processed:1;
102103
} ngx_http_modsecurity_ctx_t;
103104

104105

@@ -139,6 +140,7 @@ extern ngx_module_t ngx_http_modsecurity_module;
139140
/* ngx_http_modsecurity_module.c */
140141
int ngx_http_modsecurity_process_intervention (Transaction *transaction, ngx_http_request_t *r, ngx_int_t early_log);
141142
ngx_http_modsecurity_ctx_t *ngx_http_modsecurity_create_ctx(ngx_http_request_t *r);
143+
ngx_http_modsecurity_ctx_t *ngx_http_modsecurity_get_module_ctx(ngx_http_request_t *r);
142144
char *ngx_str_to_char(ngx_str_t a, ngx_pool_t *p);
143145
#if (NGX_PCRE2)
144146
#define ngx_http_modsecurity_pcre_malloc_init(x) NULL

‎src/ngx_http_modsecurity_header_filter.c‎

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -109,7 +109,7 @@ ngx_http_modsecurity_store_ctx_header(ngx_http_request_t *r, ngx_str_t *name, ng
109109
ngx_http_modsecurity_conf_t *mcf;
110110
ngx_http_modsecurity_header_t *hdr;
111111

112-
ctx = ngx_http_get_module_ctx(r, ngx_http_modsecurity_module);
112+
ctx = ngx_http_modsecurity_get_module_ctx(r);
113113
if (ctx == NULL || ctx->sanity_headers_out == NULL) {
114114
return NGX_ERROR;
115115
}
@@ -152,7 +152,7 @@ ngx_http_modsecurity_resolv_header_server(ngx_http_request_t *r, ngx_str_t name,
152152
ngx_str_t value;
153153

154154
clcf = ngx_http_get_module_loc_conf(r, ngx_http_core_module);
155-
ctx = ngx_http_get_module_ctx(r, ngx_http_modsecurity_module);
155+
ctx = ngx_http_modsecurity_get_module_ctx(r);
156156

157157
if (r->headers_out.server == NULL) {
158158
if (clcf->server_tokens) {
@@ -186,7 +186,7 @@ ngx_http_modsecurity_resolv_header_date(ngx_http_request_t *r, ngx_str_t name, o
186186
ngx_http_modsecurity_ctx_t *ctx = NULL;
187187
ngx_str_t date;
188188

189-
ctx = ngx_http_get_module_ctx(r, ngx_http_modsecurity_module);
189+
ctx = ngx_http_modsecurity_get_module_ctx(r);
190190

191191
if (r->headers_out.date == NULL) {
192192
date.data = ngx_cached_http_time.data;
@@ -216,7 +216,7 @@ ngx_http_modsecurity_resolv_header_content_length(ngx_http_request_t *r, ngx_str
216216
ngx_str_t value;
217217
char buf[NGX_INT64_LEN+2];
218218

219-
ctx = ngx_http_get_module_ctx(r, ngx_http_modsecurity_module);
219+
ctx = ngx_http_modsecurity_get_module_ctx(r);
220220

221221
if (r->headers_out.content_length_n > 0)
222222
{
@@ -243,7 +243,7 @@ ngx_http_modsecurity_resolv_header_content_type(ngx_http_request_t *r, ngx_str_t
243243
{
244244
ngx_http_modsecurity_ctx_t *ctx = NULL;
245245

246-
ctx = ngx_http_get_module_ctx(r, ngx_http_modsecurity_module);
246+
ctx = ngx_http_modsecurity_get_module_ctx(r);
247247

248248
if (r->headers_out.content_type.len > 0)
249249
{
@@ -270,7 +270,7 @@ ngx_http_modsecurity_resolv_header_last_modified(ngx_http_request_t *r, ngx_str_
270270
u_char buf[1024], *p;
271271
ngx_str_t value;
272272

273-
ctx = ngx_http_get_module_ctx(r, ngx_http_modsecurity_module);
273+
ctx = ngx_http_modsecurity_get_module_ctx(r);
274274

275275
if (r->headers_out.last_modified_time == -1) {
276276
return 1;
@@ -302,7 +302,7 @@ ngx_http_modsecurity_resolv_header_connection(ngx_http_request_t *r, ngx_str_t n
302302
ngx_str_t value;
303303

304304
clcf = ngx_http_get_module_loc_conf(r, ngx_http_core_module);
305-
ctx = ngx_http_get_module_ctx(r, ngx_http_modsecurity_module);
305+
ctx = ngx_http_modsecurity_get_module_ctx(r);
306306

307307
if (r->headers_out.status == NGX_HTTP_SWITCHING_PROTOCOLS) {
308308
connection = "upgrade";
@@ -353,7 +353,7 @@ ngx_http_modsecurity_resolv_header_transfer_encoding(ngx_http_request_t *r, ngx_
353353
if (r->chunked) {
354354
ngx_str_t value = ngx_string("chunked");
355355

356-
ctx = ngx_http_get_module_ctx(r, ngx_http_modsecurity_module);
356+
ctx = ngx_http_modsecurity_get_module_ctx(r);
357357

358358
#if defined(MODSECURITY_SANITY_CHECKS) && (MODSECURITY_SANITY_CHECKS)
359359
ngx_http_modsecurity_store_ctx_header(r, &name, &value);
@@ -380,7 +380,7 @@ ngx_http_modsecurity_resolv_header_vary(ngx_http_request_t *r, ngx_str_t name, o
380380
if (r->gzip_vary && clcf->gzip_vary) {
381381
ngx_str_t value = ngx_string("Accept-Encoding");
382382

383-
ctx = ngx_http_get_module_ctx(r, ngx_http_modsecurity_module);
383+
ctx = ngx_http_modsecurity_get_module_ctx(r);
384384

385385
#if defined(MODSECURITY_SANITY_CHECKS) && (MODSECURITY_SANITY_CHECKS)
386386
ngx_http_modsecurity_store_ctx_header(r, &name, &value);
@@ -422,7 +422,7 @@ ngx_http_modsecurity_header_filter(ngx_http_request_t *r)
422422

423423
/* XXX: if NOT_MODIFIED, do we need to process it at all? see xslt_header_filter() */
424424

425-
ctx = ngx_http_get_module_ctx(r, ngx_http_modsecurity_module);
425+
ctx = ngx_http_modsecurity_get_module_ctx(r);
426426

427427
dd("header filter, recovering ctx: %p", ctx);
428428

‎src/ngx_http_modsecurity_log.c‎

Lines changed: 3 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -41,17 +41,9 @@ ngx_http_modsecurity_log_handler(ngx_http_request_t *r)
4141
{
4242
ngx_pool_t *old_pool;
4343
ngx_http_modsecurity_ctx_t *ctx;
44-
ngx_http_modsecurity_conf_t *mcf;
4544

4645
dd("catching a new _log_ phase handler");
4746

48-
mcf = ngx_http_get_module_loc_conf(r, ngx_http_modsecurity_module);
49-
if (mcf == NULL || mcf->enable != 1)
50-
{
51-
dd("ModSecurity not enabled... returning");
52-
return NGX_OK;
53-
}
54-
5547
/*
5648
if (r->method != NGX_HTTP_GET &&
5749
r->method != NGX_HTTP_POST && r->method != NGX_HTTP_HEAD) {
@@ -60,13 +52,13 @@ ngx_http_modsecurity_log_handler(ngx_http_request_t *r)
6052
return NGX_OK;
6153
}
6254
*/
63-
ctx = ngx_http_get_module_ctx(r, ngx_http_modsecurity_module);
55+
ctx = ngx_http_modsecurity_get_module_ctx(r);
6456

6557
dd("recovering ctx: %p", ctx);
6658

6759
if (ctx == NULL) {
68-
dd("something really bad happened here. returning NGX_ERROR");
69-
return NGX_ERROR;
60+
dd("ModSecurity not enabled or error occurred");
61+
return NGX_OK;
7062
}
7163

7264
if (ctx->logged) {

‎src/ngx_http_modsecurity_module.c‎

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -149,7 +149,7 @@ ngx_http_modsecurity_process_intervention (Transaction *transaction, ngx_http_re
149149

150150
dd("processing intervention");
151151

152-
ctx = ngx_http_get_module_ctx(r, ngx_http_modsecurity_module);
152+
ctx = ngx_http_modsecurity_get_module_ctx(r);
153153
if (ctx == NULL)
154154
{
155155
return NGX_HTTP_INTERNAL_SERVER_ERROR;
@@ -313,6 +313,27 @@ ngx_http_modsecurity_create_ctx(ngx_http_request_t *r)
313313
return ctx;
314314
}
315315

316+
ngx_inline ngx_http_modsecurity_ctx_t *
317+
ngx_http_modsecurity_get_module_ctx(ngx_http_request_t *r)
318+
{
319+
ngx_http_modsecurity_ctx_t *ctx;
320+
ctx = ngx_http_get_module_ctx(r, ngx_http_modsecurity_module);
321+
if (ctx == NULL) {
322+
/*
323+
* refer <nginx>/src/http/modules/ngx_http_realip_module.c
324+
* if module context was reset, the original address
325+
* can still be found in the cleanup handler
326+
*/
327+
ngx_pool_cleanup_t *cln;
328+
for (cln = r->pool->cleanup; cln; cln = cln->next) {
329+
if (cln->handler == ngx_http_modsecurity_cleanup) {
330+
ctx = cln->data;
331+
break;
332+
}
333+
}
334+
}
335+
return ctx;
336+
}
316337

317338
char *
318339
ngx_conf_set_rules(ngx_conf_t *cf, ngx_command_t *cmd, void *conf)

0 commit comments

Comments
 (0)