Skip to content

Commit

Permalink
Merge pull request #344 from owasp-modsecurity/revert-334-fix_respons…
Browse files Browse the repository at this point in the history 
…e_body

Revert "fix: response body (based on #326)"
  • Loading branch information
@airween
airween authored Feb 17, 2025
2 parents fb678c5 + 67c9a6b commit 04665cc
Show file tree
Hide file tree
Showing 10 changed files with 96 additions and 176 deletions.
116 changes: 38 additions & 78 deletions src/ngx_http_modsecurity_body_filter.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,14 +33,12 @@ ngx_http_modsecurity_body_filter_init(void)

return NGX_OK;
}

ngx_int_t
ngx_http_modsecurity_body_filter(ngx_http_request_t *r, ngx_chain_t *in)
{
ngx_http_modsecurity_ctx_t *ctx = NULL;
ngx_chain_t *chain = in;
ngx_int_t ret;
ngx_pool_t *old_pool;
ngx_int_t is_request_processed = 0;
ngx_http_modsecurity_ctx_t *ctx = NULL;
#if defined(MODSECURITY_SANITY_CHECKS) && (MODSECURITY_SANITY_CHECKS)
ngx_http_modsecurity_conf_t *mcf;
ngx_list_part_t *part = &r->headers_out.headers.part;
Expand All @@ -49,18 +47,14 @@ ngx_http_modsecurity_body_filter(ngx_http_request_t *r, ngx_chain_t *in)
#endif

if (in == NULL) {
ngx_log_debug0(NGX_LOG_DEBUG_HTTP, r->connection->log, 0, "MDS input chain is null");

return ngx_http_next_body_filter(r, in);
}

/* get context for request */
ctx = ngx_http_modsecurity_get_module_ctx(r);
ctx = ngx_http_get_module_ctx(r, ngx_http_modsecurity_module);

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

if (ctx == NULL || r->filter_finalize || ctx->response_body_filtered) {
if (ctx && ctx->response_body_filtered)
r->filter_finalize = 1;
if (ctx == NULL) {
return ngx_http_next_body_filter(r, in);
}

Expand Down Expand Up @@ -146,81 +140,47 @@ ngx_http_modsecurity_body_filter(ngx_http_request_t *r, ngx_chain_t *in)
}
#endif

for (chain = in; chain != NULL; chain = chain->next) {

ngx_buf_t *copy_buf;
ngx_chain_t* copy_chain;
is_request_processed = chain->buf->last_buf;
int is_request_processed = 0;
for (; chain != NULL; chain = chain->next)
{
u_char *data = chain->buf->pos;
msc_append_response_body(ctx->modsec_transaction, data,
chain->buf->last - data);
ret = ngx_http_modsecurity_process_intervention(ctx->modsec_transaction,
r, 0);
int ret;

msc_append_response_body(ctx->modsec_transaction, data, chain->buf->last - data);
ret = ngx_http_modsecurity_process_intervention(ctx->modsec_transaction, r, 0);
if (ret > 0) {
return ngx_http_filter_finalize_request(r,
&ngx_http_modsecurity_module, ret);
}
if (!chain->buf->last_buf){
copy_chain = ngx_alloc_chain_link(r->pool);
if (copy_chain == NULL) {
return NGX_ERROR;
}
copy_buf = ngx_calloc_buf(r->pool);
if (copy_buf == NULL) {
return NGX_ERROR;
}
copy_buf->pos = chain->buf->pos ;
copy_buf->end = chain->buf->end;
copy_buf->last = chain->buf->last;
copy_buf->temporary = (chain->buf->temporary == 1) ? 1 : 0;
copy_buf->memory = (chain->buf->memory == 1) ? 1 : 0;
copy_chain->buf = copy_buf;
copy_chain->buf->last_buf = chain->buf->last_buf;
copy_chain->next = NULL;
chain->buf->pos = chain->buf->last;
}
else
copy_chain = chain;
if (ctx->temp_chain == NULL) {
ctx->temp_chain = copy_chain;
} else {
if (ctx->current_chain == NULL) {
ctx->temp_chain->next = copy_chain;
ctx->temp_chain->buf->last_buf = 0;
} else {
ctx->current_chain->next = copy_chain;
ctx->current_chain->buf->last_buf = 0;
}
ctx->current_chain = copy_chain;
}

}
/* XXX: chain->buf->last_buf || chain->buf->last_in_chain */
is_request_processed = chain->buf->last_buf;

if (is_request_processed) {
old_pool = ngx_http_modsecurity_pcre_malloc_init(r->pool);
msc_process_response_body(ctx->modsec_transaction);
ngx_http_modsecurity_pcre_malloc_done(old_pool);
ret = ngx_http_modsecurity_process_intervention(ctx->modsec_transaction, r, 0);
if (ret > 0) {
if (ret < NGX_HTTP_BAD_REQUEST && ctx->header_pt != NULL){
ctx->header_pt(r);
}
else {
ctx->response_body_filtered = 1;
return ngx_http_filter_finalize_request(r,
&ngx_http_modsecurity_module
, ret);
}
} else if (ret < 0) {
ctx->response_body_filtered = 1;
return ngx_http_filter_finalize_request(r,
if (is_request_processed) {
ngx_pool_t *old_pool;

old_pool = ngx_http_modsecurity_pcre_malloc_init(r->pool);
msc_process_response_body(ctx->modsec_transaction);
ngx_http_modsecurity_pcre_malloc_done(old_pool);

/* XXX: I don't get how body from modsec being transferred to nginx's buffer. If so - after adjusting of nginx's
XXX: body we can proceed to adjust body size (content-length). see xslt_body_filter() for example */
ret = ngx_http_modsecurity_process_intervention(ctx->modsec_transaction, r, 0);
if (ret > 0) {
return ret;
}
else if (ret < 0) {
return ngx_http_filter_finalize_request(r,
&ngx_http_modsecurity_module, NGX_HTTP_INTERNAL_SERVER_ERROR);

}
}
ctx->response_body_filtered = 1;
if (ctx->header_pt != NULL)
ctx->header_pt(r);
return ngx_http_next_body_filter(r, ctx->temp_chain);
} else {
return NGX_AGAIN;
}
if (!is_request_processed)
{
dd("buffer was not fully loaded! ctx: %p", ctx);
}

/* XXX: xflt_filter() -- return NGX_OK here */
return ngx_http_next_body_filter(r, in);
}
7 changes: 1 addition & 6 deletions src/ngx_http_modsecurity_common.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ typedef struct {
ngx_str_t value;
} ngx_http_modsecurity_header_t;


typedef struct {
ngx_http_request_t *r;
Transaction *modsec_transaction;
Expand All @@ -96,13 +97,8 @@ typedef struct {
unsigned waiting_more_body:1;
unsigned body_requested:1;
unsigned processed:1;
ngx_http_output_header_filter_pt header_pt;
ngx_chain_t* temp_chain;
ngx_chain_t* current_chain;
unsigned response_body_filtered:1;
unsigned logged:1;
unsigned intervention_triggered:1;
unsigned request_body_processed:1;
} ngx_http_modsecurity_ctx_t;


Expand Down Expand Up @@ -143,7 +139,6 @@ extern ngx_module_t ngx_http_modsecurity_module;
/* ngx_http_modsecurity_module.c */
int ngx_http_modsecurity_process_intervention (Transaction *transaction, ngx_http_request_t *r, ngx_int_t early_log);
ngx_http_modsecurity_ctx_t *ngx_http_modsecurity_create_ctx(ngx_http_request_t *r);
ngx_http_modsecurity_ctx_t *ngx_http_modsecurity_get_module_ctx(ngx_http_request_t *r);
char *ngx_str_to_char(ngx_str_t a, ngx_pool_t *p);
#if (NGX_PCRE2)
#define ngx_http_modsecurity_pcre_malloc_init(x) NULL
Expand Down
39 changes: 16 additions & 23 deletions src/ngx_http_modsecurity_header_filter.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ ngx_http_modsecurity_store_ctx_header(ngx_http_request_t *r, ngx_str_t *name, ng
ngx_http_modsecurity_conf_t *mcf;
ngx_http_modsecurity_header_t *hdr;

ctx = ngx_http_modsecurity_get_module_ctx(r);
ctx = ngx_http_get_module_ctx(r, ngx_http_modsecurity_module);
if (ctx == NULL || ctx->sanity_headers_out == NULL) {
return NGX_ERROR;
}
Expand Down Expand Up @@ -152,7 +152,7 @@ ngx_http_modsecurity_resolv_header_server(ngx_http_request_t *r, ngx_str_t name,
ngx_str_t value;

clcf = ngx_http_get_module_loc_conf(r, ngx_http_core_module);
ctx = ngx_http_modsecurity_get_module_ctx(r);
ctx = ngx_http_get_module_ctx(r, ngx_http_modsecurity_module);

if (r->headers_out.server == NULL) {
if (clcf->server_tokens) {
Expand Down Expand Up @@ -186,7 +186,7 @@ ngx_http_modsecurity_resolv_header_date(ngx_http_request_t *r, ngx_str_t name, o
ngx_http_modsecurity_ctx_t *ctx = NULL;
ngx_str_t date;

ctx = ngx_http_modsecurity_get_module_ctx(r);
ctx = ngx_http_get_module_ctx(r, ngx_http_modsecurity_module);

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

ctx = ngx_http_modsecurity_get_module_ctx(r);
ctx = ngx_http_get_module_ctx(r, ngx_http_modsecurity_module);

if (r->headers_out.content_length_n > 0)
{
Expand All @@ -243,7 +243,7 @@ ngx_http_modsecurity_resolv_header_content_type(ngx_http_request_t *r, ngx_str_t
{
ngx_http_modsecurity_ctx_t *ctx = NULL;

ctx = ngx_http_modsecurity_get_module_ctx(r);
ctx = ngx_http_get_module_ctx(r, ngx_http_modsecurity_module);

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

ctx = ngx_http_modsecurity_get_module_ctx(r);
ctx = ngx_http_get_module_ctx(r, ngx_http_modsecurity_module);

if (r->headers_out.last_modified_time == -1) {
return 1;
Expand Down Expand Up @@ -302,7 +302,7 @@ ngx_http_modsecurity_resolv_header_connection(ngx_http_request_t *r, ngx_str_t n
ngx_str_t value;

clcf = ngx_http_get_module_loc_conf(r, ngx_http_core_module);
ctx = ngx_http_modsecurity_get_module_ctx(r);
ctx = ngx_http_get_module_ctx(r, ngx_http_modsecurity_module);

if (r->headers_out.status == NGX_HTTP_SWITCHING_PROTOCOLS) {
connection = "upgrade";
Expand Down Expand Up @@ -353,7 +353,7 @@ ngx_http_modsecurity_resolv_header_transfer_encoding(ngx_http_request_t *r, ngx_
if (r->chunked) {
ngx_str_t value = ngx_string("chunked");

ctx = ngx_http_modsecurity_get_module_ctx(r);
ctx = ngx_http_get_module_ctx(r, ngx_http_modsecurity_module);

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

ctx = ngx_http_modsecurity_get_module_ctx(r);
ctx = ngx_http_get_module_ctx(r, ngx_http_modsecurity_module);

#if defined(MODSECURITY_SANITY_CHECKS) && (MODSECURITY_SANITY_CHECKS)
ngx_http_modsecurity_store_ctx_header(r, &name, &value);
Expand Down Expand Up @@ -422,7 +422,7 @@ ngx_http_modsecurity_header_filter(ngx_http_request_t *r)

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

ctx = ngx_http_modsecurity_get_module_ctx(r);
ctx = ngx_http_get_module_ctx(r, ngx_http_modsecurity_module);

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

Expand Down Expand Up @@ -551,17 +551,10 @@ ngx_http_modsecurity_header_filter(ngx_http_request_t *r)
*
*/

/*
* The line below is commented to make the spdy test to work
*/
//r->headers_out.content_length_n = -1;

if (ctx->response_body_filtered){
return ngx_http_next_header_filter(r);
}
else {
ctx->header_pt = ngx_http_next_header_filter;
r->headers_out.content_length_n = -1;
return NGX_AGAIN;
}
/*
* The line below is commented to make the spdy test to work
*/
//r->headers_out.content_length_n = -1;

return ngx_http_next_header_filter(r);
}
6 changes: 3 additions & 3 deletions src/ngx_http_modsecurity_log.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,13 +60,13 @@ ngx_http_modsecurity_log_handler(ngx_http_request_t *r)
return NGX_OK;
}
*/
ctx = ngx_http_modsecurity_get_module_ctx(r);
ctx = ngx_http_get_module_ctx(r, ngx_http_modsecurity_module);

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

if (ctx == NULL) {
dd("ModSecurity not enabled or error occurred");
return NGX_OK;
dd("something really bad happened here. returning NGX_ERROR");
return NGX_ERROR;
}

if (ctx->logged) {
Expand Down
24 changes: 1 addition & 23 deletions src/ngx_http_modsecurity_module.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ ngx_http_modsecurity_process_intervention (Transaction *transaction, ngx_http_re

dd("processing intervention");

ctx = ngx_http_modsecurity_get_module_ctx(r);
ctx = ngx_http_get_module_ctx(r, ngx_http_modsecurity_module);
if (ctx == NULL)
{
return NGX_HTTP_INTERNAL_SERVER_ERROR;
Expand Down Expand Up @@ -292,7 +292,6 @@ ngx_http_modsecurity_create_ctx(ngx_http_request_t *r)

dd("transaction created");

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

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

ngx_inline ngx_http_modsecurity_ctx_t *
ngx_http_modsecurity_get_module_ctx(ngx_http_request_t *r)
{
ngx_http_modsecurity_ctx_t *ctx;
ctx = ngx_http_get_module_ctx(r, ngx_http_modsecurity_module);
if (ctx == NULL) {
/*
* refer <nginx>/src/http/modules/ngx_http_realip_module.c
* if module context was reset, the original address
* can still be found in the cleanup handler
*/
ngx_pool_cleanup_t *cln;
for (cln = r->pool->cleanup; cln; cln = cln->next) {
if (cln->handler == ngx_http_modsecurity_cleanup) {
ctx = cln->data;
break;
}
}
}
return ctx;
}

char *
ngx_conf_set_rules(ngx_conf_t *cf, ngx_command_t *cmd, void *conf)
Expand Down
10 changes: 2 additions & 8 deletions src/ngx_http_modsecurity_pre_access.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ ngx_http_modsecurity_request_read(ngx_http_request_t *r)
{
ngx_http_modsecurity_ctx_t *ctx;

ctx = ngx_http_modsecurity_get_module_ctx(r);
ctx = ngx_http_get_module_ctx(r, ngx_http_modsecurity_module);

#if defined(nginx_version) && nginx_version >= 8011
r->main->count--;
Expand Down Expand Up @@ -70,7 +70,7 @@ ngx_http_modsecurity_pre_access_handler(ngx_http_request_t *r)
}
*/

ctx = ngx_http_modsecurity_get_module_ctx(r);
ctx = ngx_http_get_module_ctx(r, ngx_http_modsecurity_module);

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

Expand All @@ -80,11 +80,6 @@ ngx_http_modsecurity_pre_access_handler(ngx_http_request_t *r)
return NGX_HTTP_INTERNAL_SERVER_ERROR;
}

if (ctx->request_body_processed) {
// should we use r->internal or r->filter_finalize?
return NGX_DECLINED;
}

if (ctx->intervention_triggered) {
return NGX_DECLINED;
}
Expand Down Expand Up @@ -217,7 +212,6 @@ ngx_http_modsecurity_pre_access_handler(ngx_http_request_t *r)

old_pool = ngx_http_modsecurity_pcre_malloc_init(r->pool);
msc_process_request_body(ctx->modsec_transaction);
ctx->request_body_processed = 1;
ngx_http_modsecurity_pcre_malloc_done(old_pool);

ret = ngx_http_modsecurity_process_intervention(ctx->modsec_transaction, r, 0);
Expand Down
Loading

0 comments on commit 04665cc

Please sign in to comment.