Skip to content

Commit

Permalink
bugfix: prevented request smuggling in the ngx.location.capture API.
Browse files Browse the repository at this point in the history 
Signed-off-by: Yichun Zhang (agentzh) <yichun@openresty.com>
  • Loading branch information
@thibaultcha @agentzh
thibaultcha authored and agentzh committed Jul 3, 2020
1 parent 6913b1b commit 9ab38e8
Show file tree
Hide file tree
Showing 2 changed files with 585 additions and 131 deletions.
196 changes: 68 additions & 128 deletions src/ngx_http_lua_subrequest.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,8 +57,6 @@ static ngx_str_t ngx_http_lua_content_length_header_key =
ngx_string("Content-Length");


static ngx_int_t ngx_http_lua_set_content_length_header(ngx_http_request_t *r,
off_t len);
static ngx_int_t ngx_http_lua_adjust_subrequest(ngx_http_request_t *sr,
ngx_uint_t method, int forward_body,
ngx_http_request_body_t *body, unsigned vars_action,
Expand All @@ -79,7 +77,7 @@ static void ngx_http_lua_cancel_subreq(ngx_http_request_t *r);
static ngx_int_t ngx_http_post_request_to_head(ngx_http_request_t *r);
static ngx_int_t ngx_http_lua_copy_in_file_request_body(ngx_http_request_t *r);
static ngx_int_t ngx_http_lua_copy_request_headers(ngx_http_request_t *sr,
ngx_http_request_t *r);
ngx_http_request_t *pr, int pr_not_chunked);


enum {
Expand Down Expand Up @@ -634,8 +632,8 @@ ngx_http_lua_adjust_subrequest(ngx_http_request_t *sr, ngx_uint_t method,
unsigned vars_action, ngx_array_t *extra_vars)
{
ngx_http_request_t *r;
ngx_int_t rc;
ngx_http_core_main_conf_t *cmcf;
int pr_not_chunked = 0;
size_t size;

r = sr->parent;
Expand All @@ -645,46 +643,32 @@ ngx_http_lua_adjust_subrequest(ngx_http_request_t *sr, ngx_uint_t method,
if (body) {
sr->request_body = body;

rc = ngx_http_lua_set_content_length_header(sr,
body->buf
? ngx_buf_size(body->buf)
: 0);

if (rc != NGX_OK) {
return NGX_ERROR;
}

} else if (!always_forward_body
&& method != NGX_HTTP_PUT
&& method != NGX_HTTP_POST
&& r->headers_in.content_length_n > 0)
{
rc = ngx_http_lua_set_content_length_header(sr, 0);
if (rc != NGX_OK) {
return NGX_ERROR;
}

#if 1
sr->request_body = NULL;
#endif

} else {
if (ngx_http_lua_copy_request_headers(sr, r) != NGX_OK) {
return NGX_ERROR;
if (!r->headers_in.chunked) {
pr_not_chunked = 1;
}

if (sr->request_body) {
if (sr->request_body && sr->request_body->temp_file) {

/* deep-copy the request body */

if (sr->request_body->temp_file) {
if (ngx_http_lua_copy_in_file_request_body(sr) != NGX_OK) {
return NGX_ERROR;
}
if (ngx_http_lua_copy_in_file_request_body(sr) != NGX_OK) {
return NGX_ERROR;
}
}
}

if (ngx_http_lua_copy_request_headers(sr, r, pr_not_chunked) != NGX_OK) {
return NGX_ERROR;
}

sr->method = method;

switch (method) {
Expand Down Expand Up @@ -1134,100 +1118,6 @@ ngx_http_lua_post_subrequest(ngx_http_request_t *r, void *data, ngx_int_t rc)
}


static ngx_int_t
ngx_http_lua_set_content_length_header(ngx_http_request_t *r, off_t len)
{
ngx_table_elt_t *h, *header;
u_char *p;
ngx_list_part_t *part;
ngx_http_request_t *pr;
ngx_uint_t i;

r->headers_in.content_length_n = len;

if (ngx_list_init(&r->headers_in.headers, r->pool, 20,
sizeof(ngx_table_elt_t)) != NGX_OK)
{
return NGX_ERROR;
}

h = ngx_list_push(&r->headers_in.headers);
if (h == NULL) {
return NGX_ERROR;
}

h->key = ngx_http_lua_content_length_header_key;
h->lowcase_key = ngx_pnalloc(r->pool, h->key.len);
if (h->lowcase_key == NULL) {
return NGX_ERROR;
}

ngx_strlow(h->lowcase_key, h->key.data, h->key.len);

r->headers_in.content_length = h;

p = ngx_palloc(r->pool, NGX_OFF_T_LEN);
if (p == NULL) {
return NGX_ERROR;
}

h->value.data = p;

h->value.len = ngx_sprintf(h->value.data, "%O", len) - h->value.data;

h->hash = ngx_http_lua_content_length_hash;

#if 0
dd("content length hash: %lu == %lu", (unsigned long) h->hash,
ngx_hash_key_lc((u_char *) "Content-Length",
sizeof("Content-Length") - 1));
#endif

dd("r content length: %.*s",
(int) r->headers_in.content_length->value.len,
r->headers_in.content_length->value.data);

pr = r->parent;

if (pr == NULL) {
return NGX_OK;
}

/* forward the parent request's all other request headers */

part = &pr->headers_in.headers.part;
header = part->elts;

for (i = 0; /* void */; i++) {

if (i >= part->nelts) {
if (part->next == NULL) {
break;
}

part = part->next;
header = part->elts;
i = 0;
}

if (header[i].key.len == sizeof("Content-Length") - 1
&& ngx_strncasecmp(header[i].key.data, (u_char *) "Content-Length",
sizeof("Content-Length") - 1) == 0)
{
continue;
}

if (ngx_http_lua_set_input_header(r, header[i].key,
header[i].value, 0) == NGX_ERROR)
{
return NGX_ERROR;
}
}

return NGX_OK;
}


static void
ngx_http_lua_handle_subreq_responses(ngx_http_request_t *r,
ngx_http_lua_ctx_t *ctx)
Expand Down Expand Up @@ -1742,22 +1632,64 @@ ngx_http_lua_copy_in_file_request_body(ngx_http_request_t *r)


static ngx_int_t
ngx_http_lua_copy_request_headers(ngx_http_request_t *sr, ngx_http_request_t *r)
ngx_http_lua_copy_request_headers(ngx_http_request_t *sr,
ngx_http_request_t *pr, int pr_not_chunked)
{
ngx_table_elt_t *header;
ngx_table_elt_t *clh, *header;
ngx_list_part_t *part;
ngx_uint_t i;
u_char *p;
off_t len;

dd("before: parent req headers count: %d",
(int) pr->headers_in.headers.part.nelts);

if (ngx_list_init(&sr->headers_in.headers, sr->pool, 20,
sizeof(ngx_table_elt_t)) != NGX_OK)
{
return NGX_ERROR;
}

dd("before: parent req headers count: %d",
(int) r->headers_in.headers.part.nelts);
if (sr->request_body && !pr_not_chunked) {

/* craft our own Content-Length */

len = sr->request_body->buf ? ngx_buf_size(sr->request_body->buf) : 0;

clh = ngx_list_push(&sr->headers_in.headers);
if (clh == NULL) {
return NGX_ERROR;
}

part = &r->headers_in.headers.part;
clh->hash = ngx_http_lua_content_length_hash;
clh->key = ngx_http_lua_content_length_header_key;
clh->lowcase_key = ngx_pnalloc(sr->pool, clh->key.len);
if (clh->lowcase_key == NULL) {
return NGX_ERROR;
}

ngx_strlow(clh->lowcase_key, clh->key.data, clh->key.len);

p = ngx_palloc(sr->pool, NGX_OFF_T_LEN);
if (p == NULL) {
return NGX_ERROR;
}

clh->value.data = p;
clh->value.len = ngx_sprintf(clh->value.data, "%O", len)
- clh->value.data;

sr->headers_in.content_length = clh;
sr->headers_in.content_length_n = len;

dd("sr crafted content-length: %.*s",
(int) sr->headers_in.content_length->value.len,
sr->headers_in.content_length->value.data);
}

/* copy the parent request's headers */

part = &pr->headers_in.headers.part;
header = part->elts;

for (i = 0; /* void */; i++) {
Expand All @@ -1772,7 +1704,14 @@ ngx_http_lua_copy_request_headers(ngx_http_request_t *sr, ngx_http_request_t *r)
i = 0;
}

dd("setting request header %.*s: %.*s", (int) header[i].key.len,
if (!pr_not_chunked && header[i].key.len == sizeof("Content-Length") - 1
&& ngx_strncasecmp(header[i].key.data, (u_char *) "Content-Length",
sizeof("Content-Length") - 1) == 0)
{
continue;
}

dd("sr copied req header %.*s: %.*s", (int) header[i].key.len,
header[i].key.data, (int) header[i].value.len,
header[i].value.data);

Expand All @@ -1784,9 +1723,10 @@ ngx_http_lua_copy_request_headers(ngx_http_request_t *sr, ngx_http_request_t *r)
}

dd("after: parent req headers count: %d",
(int) r->headers_in.headers.part.nelts);
(int) pr->headers_in.headers.part.nelts);

return NGX_OK;
}


/* vi:set ft=c ts=4 sw=4 et fdm=marker: */
Loading

0 comments on commit 9ab38e8

Please sign in to comment.