Skip to content

Commit 62639fa

Browse files
g00g1g00g1
committed
Fix #41 
1 parent ef64996 commit 62639fa

7 files changed

+129
-78
lines changed

Diff for: ‎src/ngx_http_modsecurity_body_filter.c

+77-37
Original file line numberDiff line numberDiff line change
@@ -33,12 +33,14 @@ ngx_http_modsecurity_body_filter_init(void)
3333

3434
return NGX_OK;
3535
}
36-
3736
ngx_int_t
3837
ngx_http_modsecurity_body_filter(ngx_http_request_t *r, ngx_chain_t *in)
3938
{
40-
ngx_chain_t *chain = in;
4139
ngx_http_modsecurity_ctx_t *ctx = NULL;
40+
ngx_chain_t *chain = in;
41+
ngx_int_t ret;
42+
ngx_pool_t *old_pool;
43+
ngx_int_t is_request_processed = 0;
4244
#if defined(MODSECURITY_SANITY_CHECKS) && (MODSECURITY_SANITY_CHECKS)
4345
ngx_http_modsecurity_conf_t *mcf;
4446
ngx_list_part_t *part = &r->headers_out.headers.part;
@@ -47,14 +49,18 @@ ngx_http_modsecurity_body_filter(ngx_http_request_t *r, ngx_chain_t *in)
4749
#endif
4850

4951
if (in == NULL) {
52+
ngx_log_debug0(NGX_LOG_DEBUG_HTTP, r->connection->log, 0, "MDS input chain is null");
53+
5054
return ngx_http_next_body_filter(r, in);
5155
}
5256

57+
/* get context for request */
5358
ctx = ngx_http_get_module_ctx(r, ngx_http_modsecurity_module);
54-
5559
dd("body filter, recovering ctx: %p", ctx);
5660

57-
if (ctx == NULL) {
61+
if (ctx == NULL || r->filter_finalize || ctx->response_body_filtered) {
62+
if (ctx && ctx->response_body_filtered)
63+
r->filter_finalize = 1;
5864
return ngx_http_next_body_filter(r, in);
5965
}
6066

@@ -140,47 +146,81 @@ ngx_http_modsecurity_body_filter(ngx_http_request_t *r, ngx_chain_t *in)
140146
}
141147
#endif
142148

143-
int is_request_processed = 0;
144-
for (; chain != NULL; chain = chain->next)
145-
{
146-
u_char *data = chain->buf->pos;
147-
int ret;
149+
for (chain = in; chain != NULL; chain = chain->next) {
148150

149-
msc_append_response_body(ctx->modsec_transaction, data, chain->buf->last - data);
150-
ret = ngx_http_modsecurity_process_intervention(ctx->modsec_transaction, r, 0);
151+
ngx_buf_t *copy_buf;
152+
ngx_chain_t* copy_chain;
153+
is_request_processed = chain->buf->last_buf;
154+
u_char *data = chain->buf->pos;
155+
msc_append_response_body(ctx->modsec_transaction, data,
156+
chain->buf->last - data);
157+
ret = ngx_http_modsecurity_process_intervention(ctx->modsec_transaction,
158+
r, 0);
151159
if (ret > 0) {
152160
return ngx_http_filter_finalize_request(r,
153161
&ngx_http_modsecurity_module, ret);
154162
}
155-
156-
/* XXX: chain->buf->last_buf || chain->buf->last_in_chain */
157-
is_request_processed = chain->buf->last_buf;
158-
159-
if (is_request_processed) {
160-
ngx_pool_t *old_pool;
161-
162-
old_pool = ngx_http_modsecurity_pcre_malloc_init(r->pool);
163-
msc_process_response_body(ctx->modsec_transaction);
164-
ngx_http_modsecurity_pcre_malloc_done(old_pool);
165-
166-
/* XXX: I don't get how body from modsec being transferred to nginx's buffer. If so - after adjusting of nginx's
167-
XXX: body we can proceed to adjust body size (content-length). see xslt_body_filter() for example */
168-
ret = ngx_http_modsecurity_process_intervention(ctx->modsec_transaction, r, 0);
169-
if (ret > 0) {
170-
return ret;
163+
if (!chain->buf->last_buf){
164+
copy_chain = ngx_alloc_chain_link(r->pool);
165+
if (copy_chain == NULL) {
166+
return NGX_ERROR;
171167
}
172-
else if (ret < 0) {
173-
return ngx_http_filter_finalize_request(r,
174-
&ngx_http_modsecurity_module, NGX_HTTP_INTERNAL_SERVER_ERROR);
175-
168+
copy_buf = ngx_calloc_buf(r->pool);
169+
if (copy_buf == NULL) {
170+
return NGX_ERROR;
176171
}
172+
copy_buf->pos = chain->buf->pos ;
173+
copy_buf->end = chain->buf->end;
174+
copy_buf->last = chain->buf->last;
175+
copy_buf->temporary = (chain->buf->temporary == 1) ? 1 : 0;
176+
copy_buf->memory = (chain->buf->memory == 1) ? 1 : 0;
177+
copy_chain->buf = copy_buf;
178+
copy_chain->buf->last_buf = chain->buf->last_buf;
179+
copy_chain->next = NULL;
180+
chain->buf->pos = chain->buf->last;
177181
}
178-
}
179-
if (!is_request_processed)
180-
{
181-
dd("buffer was not fully loaded! ctx: %p", ctx);
182+
else
183+
copy_chain = chain;
184+
if (ctx->temp_chain == NULL) {
185+
ctx->temp_chain = copy_chain;
186+
} else {
187+
if (ctx->current_chain == NULL) {
188+
ctx->temp_chain->next = copy_chain;
189+
ctx->temp_chain->buf->last_buf = 0;
190+
} else {
191+
ctx->current_chain->next = copy_chain;
192+
ctx->current_chain->buf->last_buf = 0;
193+
}
194+
ctx->current_chain = copy_chain;
195+
}
196+
182197
}
183198

184-
/* XXX: xflt_filter() -- return NGX_OK here */
185-
return ngx_http_next_body_filter(r, in);
199+
if (is_request_processed) {
200+
old_pool = ngx_http_modsecurity_pcre_malloc_init(r->pool);
201+
msc_process_response_body(ctx->modsec_transaction);
202+
ngx_http_modsecurity_pcre_malloc_done(old_pool);
203+
ret = ngx_http_modsecurity_process_intervention(ctx->modsec_transaction, r, 0);
204+
if (ret > 0) {
205+
if (ret < NGX_HTTP_BAD_REQUEST && ctx->header_pt != NULL){
206+
ctx->header_pt(r);
207+
}
208+
else {
209+
ctx->response_body_filtered = 1;
210+
return ngx_http_filter_finalize_request(r,
211+
&ngx_http_modsecurity_module
212+
, ret);
213+
}
214+
} else if (ret < 0) {
215+
ctx->response_body_filtered = 1;
216+
return ngx_http_filter_finalize_request(r,
217+
&ngx_http_modsecurity_module, NGX_HTTP_INTERNAL_SERVER_ERROR);
218+
}
219+
ctx->response_body_filtered = 1;
220+
if (ctx->header_pt != NULL)
221+
ctx->header_pt(r);
222+
return ngx_http_next_body_filter(r, ctx->temp_chain);
223+
} else {
224+
return NGX_AGAIN;
225+
}
186226
}

Diff for: ‎src/ngx_http_modsecurity_common.h

+4-1
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,6 @@ typedef struct {
7676
ngx_str_t value;
7777
} ngx_http_modsecurity_header_t;
7878

79-
8079
typedef struct {
8180
ngx_http_request_t *r;
8281
Transaction *modsec_transaction;
@@ -97,6 +96,10 @@ typedef struct {
9796
unsigned waiting_more_body:1;
9897
unsigned body_requested:1;
9998
unsigned processed:1;
99+
ngx_http_output_header_filter_pt header_pt;
100+
ngx_chain_t* temp_chain;
101+
ngx_chain_t* current_chain;
102+
unsigned response_body_filtered:1;
100103
unsigned logged:1;
101104
unsigned intervention_triggered:1;
102105
} ngx_http_modsecurity_ctx_t;

Diff for: ‎src/ngx_http_modsecurity_header_filter.c

+13-6
Original file line numberDiff line numberDiff line change
@@ -551,10 +551,17 @@ ngx_http_modsecurity_header_filter(ngx_http_request_t *r)
551551
*
552552
*/
553553

554-
/*
555-
* The line below is commented to make the spdy test to work
556-
*/
557-
//r->headers_out.content_length_n = -1;
558-
559-
return ngx_http_next_header_filter(r);
554+
/*
555+
* The line below is commented to make the spdy test to work
556+
*/
557+
//r->headers_out.content_length_n = -1;
558+
559+
if (ctx->response_body_filtered){
560+
return ngx_http_next_header_filter(r);
561+
}
562+
else {
563+
ctx->header_pt = ngx_http_next_header_filter;
564+
r->headers_out.content_length_n = -1;
565+
return NGX_AGAIN;
566+
}
560567
}

Diff for: ‎src/ngx_http_modsecurity_module.c

+1
Original file line numberDiff line numberDiff line change
@@ -292,6 +292,7 @@ ngx_http_modsecurity_create_ctx(ngx_http_request_t *r)
292292

293293
dd("transaction created");
294294

295+
ctx->response_body_filtered = 0;
295296
ngx_http_set_ctx(r, ctx, ngx_http_modsecurity_module);
296297

297298
cln = ngx_pool_cleanup_add(r->pool, sizeof(ngx_http_modsecurity_ctx_t));

Diff for: ‎tests/modsecurity-proxy.t

+16-16
Original file line numberDiff line numberDiff line change
@@ -114,28 +114,28 @@ unlike(http_head('/'), qr/SEE-THIS/, 'proxy head request');
114114

115115

116116
# Redirect (302)
117-
like(http_get('/phase1?what=redirect302'), qr/^HTTP.*302/, 'redirect 302 - phase 1');
118-
like(http_get('/phase2?what=redirect302'), qr/^HTTP.*302/, 'redirect 302 - phase 2');
119-
like(http_get('/phase3?what=redirect302'), qr/^HTTP.*302/, 'redirect 302 - phase 3');
120-
is(http_get('/phase4?what=redirect302'), '', 'redirect 302 - phase 4');
117+
like(http_get('/phase1?what=redirect302'), qr/302 Moved Temporarily/, 'redirect 302 - phase 1');
118+
like(http_get('/phase2?what=redirect302'), qr/302 Moved Temporarily/, 'redirect 302 - phase 2');
119+
like(http_get('/phase3?what=redirect302'), qr/302 Moved Temporarily/, 'redirect 302 - phase 3');
120+
like(http_get('/phase4?what=redirect302'), qr/404/, 'redirect 302 - phase 4');
121121

122122
# Redirect (301)
123-
like(http_get('/phase1?what=redirect301'), qr/^HTTP.*301/, 'redirect 301 - phase 1');
124-
like(http_get('/phase2?what=redirect301'), qr/^HTTP.*301/, 'redirect 301 - phase 2');
125-
like(http_get('/phase3?what=redirect301'), qr/^HTTP.*301/, 'redirect 301 - phase 3');
126-
is(http_get('/phase4?what=redirect301'), '', 'redirect 301 - phase 4');
123+
like(http_get('/phase1?what=redirect301'), qr/301 Moved Permanently/, 'redirect 301 - phase 1');
124+
like(http_get('/phase2?what=redirect301'), qr/301 Moved Permanently/, 'redirect 301 - phase 2');
125+
like(http_get('/phase3?what=redirect301'), qr/301 Moved Permanently/, 'redirect 301 - phase 3');
126+
like(http_get('/phase4?what=redirect301'), qr/404/, 'redirect 301 - phase 4');
127127

128128
# Block (401)
129-
like(http_get('/phase1?what=block401'), qr/^HTTP.*401/, 'block 401 - phase 1');
130-
like(http_get('/phase2?what=block401'), qr/^HTTP.*401/, 'block 401 - phase 2');
131-
like(http_get('/phase3?what=block401'), qr/^HTTP.*401/, 'block 401 - phase 3');
132-
is(http_get('/phase4?what=block401'), '', 'block 401 - phase 4');
129+
like(http_get('/phase1?what=block401'), qr/401 Unauthorized/, 'block 401 - phase 1');
130+
like(http_get('/phase2?what=block401'), qr/401 Unauthorized/, 'block 401 - phase 2');
131+
like(http_get('/phase3?what=block401'), qr/401 Unauthorized/, 'block 401 - phase 3');
132+
like(http_get('/phase4?what=block401'), qr/401 Unauthorized/, 'block 401 - phase 4');
133133

134134
# Block (403)
135-
like(http_get('/phase1?what=block403'), qr/^HTTP.*403/, 'block 403 - phase 1');
136-
like(http_get('/phase2?what=block403'), qr/^HTTP.*403/, 'block 403 - phase 2');
137-
like(http_get('/phase3?what=block403'), qr/^HTTP.*403/, 'block 403 - phase 3');
138-
is(http_get('/phase4?what=block403'), '', 'block 403 - phase 4');
135+
like(http_get('/phase1?what=block403'), qr/403 Forbidden/, 'block 403 - phase 1');
136+
like(http_get('/phase2?what=block403'), qr/403 Forbidden/, 'block 403 - phase 2');
137+
like(http_get('/phase3?what=block403'), qr/403 Forbidden/, 'block 403 - phase 3');
138+
like(http_get('/phase4?what=block403'), qr/403 Forbidden/, 'block 403 - phase 4');
139139

140140
# Nothing to detect
141141
like(http_get('/phase1?what=nothing'), qr/phase1\?what=nothing\' not found/, 'nothing phase 1');

Diff for: ‎tests/modsecurity-scoring.t

+2-2
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ http {
4646
SecRuleEngine On
4747
SecRule ARGS "@streq badarg1" "id:11,phase:2,setvar:tx.score=1"
4848
SecRule ARGS "@streq badarg2" "id:12,phase:2,setvar:tx.score=2"
49-
SecRule TX:SCORE "@ge 2" "id:199,phase:request,deny,log,status:403"
49+
SecRule tx:score "@ge 2" "id:199,phase:request,deny,log,status:403"
5050
';
5151
}
5252
@@ -56,7 +56,7 @@ http {
5656
SecRule ARGS "@streq badarg1" "id:21,phase:2,setvar:tx.score=+1"
5757
SecRule ARGS "@streq badarg2" "id:22,phase:2,setvar:tx.score=+1"
5858
SecRule ARGS "@streq badarg3" "id:23,phase:2,setvar:tx.score=+1"
59-
SecRule TX:SCORE "@ge 3" "id:299,phase:request,deny,log,status:403"
59+
SecRule tx:score "@ge 3" "id:299,phase:request,deny,log,status:403"
6060
';
6161
}
6262
}

Diff for: ‎tests/modsecurity.t

+16-16
Original file line numberDiff line numberDiff line change
@@ -139,28 +139,28 @@ $t->plan(21);
139139

140140

141141
# Redirect (302)
142-
like(http_get('/phase1?what=redirect302'), qr/^HTTP.*302/, 'redirect 302 - phase 1');
143-
like(http_get('/phase2?what=redirect302'), qr/^HTTP.*302/, 'redirect 302 - phase 2');
144-
like(http_get('/phase3?what=redirect302'), qr/^HTTP.*302/, 'redirect 302 - phase 3');
145-
is(http_get('/phase4?what=redirect302'), '', 'redirect 302 - phase 4');
142+
like(http_get('/phase1?what=redirect302'), qr/302 Moved Temporarily/, 'redirect 302 - phase 1');
143+
like(http_get('/phase2?what=redirect302'), qr/302 Moved Temporarily/, 'redirect 302 - phase 2');
144+
like(http_get('/phase3?what=redirect302'), qr/302 Moved Temporarily/, 'redirect 302 - phase 3');
145+
like(http_get('/phase4?what=redirect302'), qr/200/, 'redirect 302 - phase 4');
146146

147147
# Redirect (301)
148-
like(http_get('/phase1?what=redirect301'), qr/^HTTP.*301/, 'redirect 301 - phase 1');
149-
like(http_get('/phase2?what=redirect301'), qr/^HTTP.*301/, 'redirect 301 - phase 2');
150-
like(http_get('/phase3?what=redirect301'), qr/^HTTP.*301/, 'redirect 301 - phase 3');
151-
is(http_get('/phase4?what=redirect301'), '', 'redirect 301 - phase 4');
148+
like(http_get('/phase1?what=redirect301'), qr/301 Moved Permanently/, 'redirect 301 - phase 1');
149+
like(http_get('/phase2?what=redirect301'), qr/301 Moved Permanently/, 'redirect 301 - phase 2');
150+
like(http_get('/phase3?what=redirect301'), qr/301 Moved Permanently/, 'redirect 301 - phase 3');
151+
like(http_get('/phase4?what=redirect301'), qr/200/, 'redirect 301 - phase 4');
152152

153153
# Block (401)
154-
like(http_get('/phase1?what=block401'), qr/^HTTP.*401/, 'block 401 - phase 1');
155-
like(http_get('/phase2?what=block401'), qr/^HTTP.*401/, 'block 401 - phase 2');
156-
like(http_get('/phase3?what=block401'), qr/^HTTP.*401/, 'block 401 - phase 3');
157-
is(http_get('/phase4?what=block401'), '', 'block 401 - phase 4');
154+
like(http_get('/phase1?what=block401'), qr/401 Unauthorized/, 'block 401 - phase 1');
155+
like(http_get('/phase2?what=block401'), qr/401 Unauthorized/, 'block 401 - phase 2');
156+
like(http_get('/phase3?what=block401'), qr/401 Unauthorized/, 'block 401 - phase 3');
157+
like(http_get('/phase4?what=block401'), qr/401 Unauthorized/, 'block 401 - phase 4');
158158

159159
# Block (403)
160-
like(http_get('/phase1?what=block403'), qr/^HTTP.*403/, 'block 403 - phase 1');
161-
like(http_get('/phase2?what=block403'), qr/^HTTP.*403/, 'block 403 - phase 2');
162-
like(http_get('/phase3?what=block403'), qr/^HTTP.*403/, 'block 403 - phase 3');
163-
is(http_get('/phase4?what=block403'), '', 'block 403 - phase 4');
160+
like(http_get('/phase1?what=block403'), qr/403 Forbidden/, 'block 403 - phase 1');
161+
like(http_get('/phase2?what=block403'), qr/403 Forbidden/, 'block 403 - phase 2');
162+
like(http_get('/phase3?what=block403'), qr/403 Forbidden/, 'block 403 - phase 3');
163+
like(http_get('/phase4?what=block403'), qr/403 Forbidden/, 'block 403 - phase 4');
164164

165165
# Nothing to detect
166166
like(http_get('/phase1?what=nothing'), qr/should be moved\/blocked before this./, 'nothing phase 1');

0 commit comments

Comments
 (0)