Skip to content

Commit 04665cc

Browse files
airweenairween
authored
Merge pull request owasp-modsecurity#344 from owasp-modsecurity/revert-334-fix_response_body
Revert "fix: response body (based on owasp-modsecurity#326)"
2 parents fb678c5 + 67c9a6b commit 04665cc

10 files changed

+96
-176
lines changed

‎src/ngx_http_modsecurity_body_filter.c‎

Lines changed: 38 additions & 78 deletions
Original file line numberDiff line numberDiff line change
@@ -33,14 +33,12 @@ ngx_http_modsecurity_body_filter_init(void)
3333

3434
return NGX_OK;
3535
}
36+
3637
ngx_int_t
3738
ngx_http_modsecurity_body_filter(ngx_http_request_t *r, ngx_chain_t *in)
3839
{
39-
ngx_http_modsecurity_ctx_t *ctx = NULL;
4040
ngx_chain_t *chain = in;
41-
ngx_int_t ret;
42-
ngx_pool_t *old_pool;
43-
ngx_int_t is_request_processed = 0;
41+
ngx_http_modsecurity_ctx_t *ctx = NULL;
4442
#if defined(MODSECURITY_SANITY_CHECKS) && (MODSECURITY_SANITY_CHECKS)
4543
ngx_http_modsecurity_conf_t *mcf;
4644
ngx_list_part_t *part = &r->headers_out.headers.part;
@@ -49,18 +47,14 @@ ngx_http_modsecurity_body_filter(ngx_http_request_t *r, ngx_chain_t *in)
4947
#endif
5048

5149
if (in == NULL) {
52-
ngx_log_debug0(NGX_LOG_DEBUG_HTTP, r->connection->log, 0, "MDS input chain is null");
53-
5450
return ngx_http_next_body_filter(r, in);
5551
}
5652

57-
/* get context for request */
58-
ctx = ngx_http_modsecurity_get_module_ctx(r);
53+
ctx = ngx_http_get_module_ctx(r, ngx_http_modsecurity_module);
54+
5955
dd("body filter, recovering ctx: %p", ctx);
6056

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

@@ -146,81 +140,47 @@ ngx_http_modsecurity_body_filter(ngx_http_request_t *r, ngx_chain_t *in)
146140
}
147141
#endif
148142

149-
for (chain = in; chain != NULL; chain = chain->next) {
150-
151-
ngx_buf_t *copy_buf;
152-
ngx_chain_t* copy_chain;
153-
is_request_processed = chain->buf->last_buf;
143+
int is_request_processed = 0;
144+
for (; chain != NULL; chain = chain->next)
145+
{
154146
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);
147+
int ret;
148+
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);
159151
if (ret > 0) {
160152
return ngx_http_filter_finalize_request(r,
161153
&ngx_http_modsecurity_module, ret);
162154
}
163-
if (!chain->buf->last_buf){
164-
copy_chain = ngx_alloc_chain_link(r->pool);
165-
if (copy_chain == NULL) {
166-
return NGX_ERROR;
167-
}
168-
copy_buf = ngx_calloc_buf(r->pool);
169-
if (copy_buf == NULL) {
170-
return NGX_ERROR;
171-
}
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;
181-
}
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-
}
196155

197-
}
156+
/* XXX: chain->buf->last_buf || chain->buf->last_in_chain */
157+
is_request_processed = chain->buf->last_buf;
198158

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,
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;
171+
}
172+
else if (ret < 0) {
173+
return ngx_http_filter_finalize_request(r,
217174
&ngx_http_modsecurity_module, NGX_HTTP_INTERNAL_SERVER_ERROR);
175+
176+
}
218177
}
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;
225178
}
179+
if (!is_request_processed)
180+
{
181+
dd("buffer was not fully loaded! ctx: %p", ctx);
182+
}
183+
184+
/* XXX: xflt_filter() -- return NGX_OK here */
185+
return ngx_http_next_body_filter(r, in);
226186
}

‎src/ngx_http_modsecurity_common.h‎

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,7 @@ typedef struct {
7676
ngx_str_t value;
7777
} ngx_http_modsecurity_header_t;
7878

79+
7980
typedef struct {
8081
ngx_http_request_t *r;
8182
Transaction *modsec_transaction;
@@ -96,13 +97,8 @@ typedef struct {
9697
unsigned waiting_more_body:1;
9798
unsigned body_requested:1;
9899
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;
103100
unsigned logged:1;
104101
unsigned intervention_triggered:1;
105-
unsigned request_body_processed:1;
106102
} ngx_http_modsecurity_ctx_t;
107103

108104

@@ -143,7 +139,6 @@ extern ngx_module_t ngx_http_modsecurity_module;
143139
/* ngx_http_modsecurity_module.c */
144140
int ngx_http_modsecurity_process_intervention (Transaction *transaction, ngx_http_request_t *r, ngx_int_t early_log);
145141
ngx_http_modsecurity_ctx_t *ngx_http_modsecurity_create_ctx(ngx_http_request_t *r);
146-
ngx_http_modsecurity_ctx_t *ngx_http_modsecurity_get_module_ctx(ngx_http_request_t *r);
147142
char *ngx_str_to_char(ngx_str_t a, ngx_pool_t *p);
148143
#if (NGX_PCRE2)
149144
#define ngx_http_modsecurity_pcre_malloc_init(x) NULL

‎src/ngx_http_modsecurity_header_filter.c‎

Lines changed: 16 additions & 23 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_modsecurity_get_module_ctx(r);
112+
ctx = ngx_http_get_module_ctx(r, ngx_http_modsecurity_module);
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_modsecurity_get_module_ctx(r);
155+
ctx = ngx_http_get_module_ctx(r, ngx_http_modsecurity_module);
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_modsecurity_get_module_ctx(r);
189+
ctx = ngx_http_get_module_ctx(r, ngx_http_modsecurity_module);
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_modsecurity_get_module_ctx(r);
219+
ctx = ngx_http_get_module_ctx(r, ngx_http_modsecurity_module);
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_modsecurity_get_module_ctx(r);
246+
ctx = ngx_http_get_module_ctx(r, ngx_http_modsecurity_module);
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_modsecurity_get_module_ctx(r);
273+
ctx = ngx_http_get_module_ctx(r, ngx_http_modsecurity_module);
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_modsecurity_get_module_ctx(r);
305+
ctx = ngx_http_get_module_ctx(r, ngx_http_modsecurity_module);
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_modsecurity_get_module_ctx(r);
356+
ctx = ngx_http_get_module_ctx(r, ngx_http_modsecurity_module);
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_modsecurity_get_module_ctx(r);
383+
ctx = ngx_http_get_module_ctx(r, ngx_http_modsecurity_module);
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_modsecurity_get_module_ctx(r);
425+
ctx = ngx_http_get_module_ctx(r, ngx_http_modsecurity_module);
426426

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

@@ -551,17 +551,10 @@ 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-
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-
}
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);
567560
}

‎src/ngx_http_modsecurity_log.c‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -60,13 +60,13 @@ ngx_http_modsecurity_log_handler(ngx_http_request_t *r)
6060
return NGX_OK;
6161
}
6262
*/
63-
ctx = ngx_http_modsecurity_get_module_ctx(r);
63+
ctx = ngx_http_get_module_ctx(r, ngx_http_modsecurity_module);
6464

6565
dd("recovering ctx: %p", ctx);
6666

6767
if (ctx == NULL) {
68-
dd("ModSecurity not enabled or error occurred");
69-
return NGX_OK;
68+
dd("something really bad happened here. returning NGX_ERROR");
69+
return NGX_ERROR;
7070
}
7171

7272
if (ctx->logged) {

‎src/ngx_http_modsecurity_module.c‎

Lines changed: 1 addition & 23 deletions
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_modsecurity_get_module_ctx(r);
152+
ctx = ngx_http_get_module_ctx(r, ngx_http_modsecurity_module);
153153
if (ctx == NULL)
154154
{
155155
return NGX_HTTP_INTERNAL_SERVER_ERROR;
@@ -292,7 +292,6 @@ ngx_http_modsecurity_create_ctx(ngx_http_request_t *r)
292292

293293
dd("transaction created");
294294

295-
ctx->response_body_filtered = 0;
296295
ngx_http_set_ctx(r, ctx, ngx_http_modsecurity_module);
297296

298297
cln = ngx_pool_cleanup_add(r->pool, sizeof(ngx_http_modsecurity_ctx_t));
@@ -314,27 +313,6 @@ ngx_http_modsecurity_create_ctx(ngx_http_request_t *r)
314313
return ctx;
315314
}
316315

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

339317
char *
340318
ngx_conf_set_rules(ngx_conf_t *cf, ngx_command_t *cmd, void *conf)

‎src/ngx_http_modsecurity_pre_access.c‎

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ ngx_http_modsecurity_request_read(ngx_http_request_t *r)
2727
{
2828
ngx_http_modsecurity_ctx_t *ctx;
2929

30-
ctx = ngx_http_modsecurity_get_module_ctx(r);
30+
ctx = ngx_http_get_module_ctx(r, ngx_http_modsecurity_module);
3131

3232
#if defined(nginx_version) && nginx_version >= 8011
3333
r->main->count--;
@@ -70,7 +70,7 @@ ngx_http_modsecurity_pre_access_handler(ngx_http_request_t *r)
7070
}
7171
*/
7272

73-
ctx = ngx_http_modsecurity_get_module_ctx(r);
73+
ctx = ngx_http_get_module_ctx(r, ngx_http_modsecurity_module);
7474

7575
dd("recovering ctx: %p", ctx);
7676

@@ -80,11 +80,6 @@ ngx_http_modsecurity_pre_access_handler(ngx_http_request_t *r)
8080
return NGX_HTTP_INTERNAL_SERVER_ERROR;
8181
}
8282

83-
if (ctx->request_body_processed) {
84-
// should we use r->internal or r->filter_finalize?
85-
return NGX_DECLINED;
86-
}
87-
8883
if (ctx->intervention_triggered) {
8984
return NGX_DECLINED;
9085
}
@@ -217,7 +212,6 @@ ngx_http_modsecurity_pre_access_handler(ngx_http_request_t *r)
217212

218213
old_pool = ngx_http_modsecurity_pcre_malloc_init(r->pool);
219214
msc_process_request_body(ctx->modsec_transaction);
220-
ctx->request_body_processed = 1;
221215
ngx_http_modsecurity_pcre_malloc_done(old_pool);
222216

223217
ret = ngx_http_modsecurity_process_intervention(ctx->modsec_transaction, r, 0);

0 commit comments

Comments
 (0)