From: Dmitry Volyntsev Date: Fri, 12 Jun 2026 03:57:14 +0000 (-0700) Subject: HTTP: fix segfault reading a request header with no cache slot X-Git-Tag: 1.0.0~9 X-Git-Url: http://git.kaiwu.me/postgresql/log/contrib/postgres_fdw/stylesheets/print.css?a=commitdiff_plain;h=241dffa95d8ec2b237b1cf19eb105e0a824874fc;p=njs.git HTTP: fix segfault reading a request header with no cache slot Reading a request header via r.headersIn[name] could crash the worker with SIGSEGV. The header in question was "Proxy-Connection", but any header that nginx registers without a dedicated ngx_table_elt_t * slot was affected. The getter mapped an arbitrary header name through ngx_hash_find() and treated (char *) &r->headers_in + hh->offset as a pointer to a cache slot. This only holds for headers whose parser stores into such a slot. "Proxy-Connection" is registered in ngx_http_headers_in[] with offset 0 and a handler that stores nothing, so the getter aliased the ngx_list_t at the start of ngx_http_headers_in_t and followed its elts pointer as a bogus ngx_table_elt_t. The fix is to walk the parsed header list by name, as it is always safe. The single-value and semicolon-join flags moved into the per-name header table, and the now-unused slot pointer argument and its branch were dropped from the generic getter. The slot lookup that is removed was only an optimization for the names in ngx_http_headers_in[]; for any other header it missed the hash and walked the list anyway, so dropping it is not a performance regression. This closes #1071 issue on Github. --- diff --git a/nginx/ngx_http_js_module.c b/nginx/ngx_http_js_module.c index 67e21247..bba6a44c 100644 --- a/nginx/ngx_http_js_module.c +++ b/nginx/ngx_http_js_module.c @@ -389,8 +389,8 @@ static njs_int_t ngx_http_js_header_out_special(njs_vm_t *vm, ngx_http_request_t *r, njs_str_t *v, njs_value_t *setval, njs_value_t *retval, ngx_table_elt_t **hh); static njs_int_t ngx_http_js_header_generic(njs_vm_t *vm, - ngx_http_request_t *r, ngx_list_t *headers, ngx_table_elt_t **ph, - unsigned flags, njs_str_t *name, njs_value_t *retval); + ngx_http_request_t *r, ngx_list_t *headers, unsigned flags, + njs_str_t *name, njs_value_t *retval); #endif static njs_int_t ngx_http_js_content_encoding(njs_vm_t *vm, ngx_http_request_t *r, unsigned flags, njs_str_t *name, @@ -4393,20 +4393,24 @@ static njs_int_t ngx_http_js_ext_header_in(njs_vm_t *vm, njs_object_prop_t *prop, uint32_t atom_id, njs_value_t *value, njs_value_t *unused, njs_value_t *retval) { - unsigned flags; - njs_int_t rc; - njs_str_t name, *h; - ngx_http_request_t *r; + unsigned flags; + njs_int_t rc; + njs_str_t name; + ngx_http_request_t *r; + ngx_http_js_header_t *h; - static njs_str_t single_headers_in[] = { - njs_str("Content-Type"), - njs_str("ETag"), - njs_str("From"), - njs_str("Max-Forwards"), - njs_str("Referer"), - njs_str("Proxy-Authorization"), - njs_str("User-Agent"), - njs_str(""), + static ngx_http_js_header_t headers_in[] = { +#define header(name, fl) { njs_str(name), fl, 0 } + header("Content-Type", NJS_HEADER_SINGLE), + header("ETag", NJS_HEADER_SINGLE), + header("From", NJS_HEADER_SINGLE), + header("Max-Forwards", NJS_HEADER_SINGLE), + header("Referer", NJS_HEADER_SINGLE), + header("Proxy-Authorization", NJS_HEADER_SINGLE), + header("User-Agent", NJS_HEADER_SINGLE), + header("Cookie", NJS_HEADER_SEMICOLON), + header("", 0), +#undef header }; r = njs_vm_external(vm, ngx_http_js_request_proto_id, value); @@ -4429,11 +4433,11 @@ ngx_http_js_ext_header_in(njs_vm_t *vm, njs_object_prop_t *prop, uint32_t atom_i flags = 0; - for (h = single_headers_in; h->length > 0; h++) { - if (h->length == name.length - && ngx_strncasecmp(h->start, name.start, name.length) == 0) + for (h = headers_in; h->name.len > 0; h++) { + if (h->name.len == name.length + && ngx_strncasecmp(h->name.data, name.start, name.length) == 0) { - flags |= NJS_HEADER_SINGLE; + flags = h->flags; break; } } @@ -5142,48 +5146,11 @@ static njs_int_t ngx_http_js_header_in(njs_vm_t *vm, ngx_http_request_t *r, unsigned flags, njs_str_t *name, njs_value_t *retval) { - u_char *lowcase_key; - ngx_uint_t hash; - ngx_table_elt_t **ph; - ngx_http_header_t *hh; - ngx_http_core_main_conf_t *cmcf; - u_char storage[128]; - if (retval == NULL) { return NJS_OK; } - /* look up hashed headers */ - - if (name->length < sizeof(storage)) { - lowcase_key = storage; - - } else { - lowcase_key = ngx_pnalloc(r->pool, name->length); - if (lowcase_key == NULL) { - njs_vm_memory_error(vm); - return NJS_ERROR; - } - } - - hash = ngx_hash_strlow(lowcase_key, name->start, name->length); - - cmcf = ngx_http_get_module_main_conf(r, ngx_http_core_module); - - hh = ngx_hash_find(&cmcf->headers_in_hash, hash, lowcase_key, - name->length); - - ph = NULL; - - if (hh) { - if (hh->offset == offsetof(ngx_http_headers_in_t, cookie)) { - flags |= NJS_HEADER_SEMICOLON; - } - - ph = (ngx_table_elt_t **) ((char *) &r->headers_in + hh->offset); - } - - return ngx_http_js_header_generic(vm, r, &r->headers_in.headers, ph, flags, + return ngx_http_js_header_generic(vm, r, &r->headers_in.headers, flags, name, retval); } @@ -5203,9 +5170,8 @@ ngx_http_js_header_out(njs_vm_t *vm, ngx_http_request_t *r, unsigned flags, njs_opaque_value_t lvalue; if (retval != NULL && setval == NULL) { - return ngx_http_js_header_generic(vm, r, &r->headers_out.headers, NULL, - flags, name, retval); - + return ngx_http_js_header_generic(vm, r, &r->headers_out.headers, flags, + name, retval); } part = &r->headers_out.headers.part; @@ -5430,8 +5396,7 @@ done: static njs_int_t ngx_http_js_header_generic(njs_vm_t *vm, ngx_http_request_t *r, - ngx_list_t *headers, ngx_table_elt_t **ph, unsigned flags, njs_str_t *name, - njs_value_t *retval) + ngx_list_t *headers, unsigned flags, njs_str_t *name, njs_value_t *retval) { u_char sep; njs_chb_t chain; @@ -5439,43 +5404,38 @@ ngx_http_js_header_generic(njs_vm_t *vm, ngx_http_request_t *r, ngx_uint_t i; njs_value_t *value; ngx_list_part_t *part; - ngx_table_elt_t *header, *h; - - if (ph == NULL) { - /* iterate over all headers */ - - ph = &header; - part = &headers->part; - h = part->elts; + ngx_table_elt_t *header, *h, **ph; - for (i = 0; /* void */ ; i++) { + ph = &header; + part = &headers->part; + h = part->elts; - if (i >= part->nelts) { - if (part->next == NULL) { - break; - } + for (i = 0; /* void */ ; i++) { - part = part->next; - h = part->elts; - i = 0; + if (i >= part->nelts) { + if (part->next == NULL) { + break; } - if (h[i].hash == 0 - || name->length != h[i].key.len - || ngx_strncasecmp(name->start, h[i].key.data, name->length) - != 0) - { - continue; - } + part = part->next; + h = part->elts; + i = 0; + } - *ph = &h[i]; - ph = &h[i].next; + if (h[i].hash == 0 + || name->length != h[i].key.len + || ngx_strncasecmp(name->start, h[i].key.data, name->length) != 0) + { + continue; } - *ph = NULL; - ph = &header; + *ph = &h[i]; + ph = &h[i].next; } + *ph = NULL; + ph = &header; + if (*ph == NULL) { njs_value_undefined_set(retval); return NJS_DECLINED; @@ -8375,51 +8335,46 @@ ngx_http_qjs_headers_in_own_property_names(JSContext *cx, static njs_int_t ngx_http_qjs_header_generic(JSContext *cx, ngx_http_request_t *r, - ngx_list_t *headers, ngx_table_elt_t **ph, ngx_str_t *name, - JSPropertyDescriptor *pdesc, unsigned flags) + ngx_list_t *headers, ngx_str_t *name, JSPropertyDescriptor *pdesc, + unsigned flags) { u_char sep; JSValue val; njs_chb_t chain; ngx_uint_t i; ngx_list_part_t *part; - ngx_table_elt_t *header, *h; - - if (ph == NULL) { - /* iterate over all headers */ - - ph = &header; - part = &headers->part; - h = part->elts; + ngx_table_elt_t *header, *h, **ph; - for (i = 0; /* void */ ; i++) { + ph = &header; + part = &headers->part; + h = part->elts; - if (i >= part->nelts) { - if (part->next == NULL) { - break; - } + for (i = 0; /* void */ ; i++) { - part = part->next; - h = part->elts; - i = 0; + if (i >= part->nelts) { + if (part->next == NULL) { + break; } - if (h[i].hash == 0 - || name->len != h[i].key.len - || ngx_strncasecmp(name->data, h[i].key.data, name->len) - != 0) - { - continue; - } + part = part->next; + h = part->elts; + i = 0; + } - *ph = &h[i]; - ph = &h[i].next; + if (h[i].hash == 0 + || name->len != h[i].key.len + || ngx_strncasecmp(name->data, h[i].key.data, name->len) != 0) + { + continue; } - *ph = NULL; - ph = &header; + *ph = &h[i]; + ph = &h[i].next; } + *ph = NULL; + ph = &header; + if (*ph == NULL) { return 0; } @@ -8500,44 +8455,7 @@ static int ngx_http_qjs_header_in(JSContext *cx, ngx_http_request_t *r, unsigned flags, ngx_str_t *name, JSPropertyDescriptor *pdesc) { - u_char *lowcase_key; - ngx_uint_t hash; - ngx_table_elt_t **ph; - ngx_http_header_t *hh; - ngx_http_core_main_conf_t *cmcf; - u_char storage[128]; - - /* look up hashed headers */ - - if (name->len < sizeof(storage)) { - lowcase_key = storage; - - } else { - lowcase_key = ngx_pnalloc(r->pool, name->len); - if (lowcase_key == NULL) { - (void) JS_ThrowOutOfMemory(cx); - return -1; - } - } - - hash = ngx_hash_strlow(lowcase_key, name->data, name->len); - - cmcf = ngx_http_get_module_main_conf(r, ngx_http_core_module); - - hh = ngx_hash_find(&cmcf->headers_in_hash, hash, lowcase_key, - name->len); - - ph = NULL; - - if (hh) { - if (hh->offset == offsetof(ngx_http_headers_in_t, cookie)) { - flags |= NJS_HEADER_SEMICOLON; - } - - ph = (ngx_table_elt_t **) ((char *) &r->headers_in + hh->offset); - } - - return ngx_http_qjs_header_generic(cx, r, &r->headers_in.headers, ph, name, + return ngx_http_qjs_header_generic(cx, r, &r->headers_in.headers, name, pdesc, flags); } @@ -8546,20 +8464,24 @@ static int ngx_http_qjs_headers_in_own_property(JSContext *cx, JSPropertyDescriptor *pdesc, JSValueConst obj, JSAtom prop) { - int ret; - unsigned flags; - ngx_str_t name, *h; - ngx_http_request_t *r; + int ret; + unsigned flags; + ngx_str_t name; + ngx_http_request_t *r; + ngx_http_js_header_t *h; - static ngx_str_t single_headers_in[] = { - ngx_string("Content-Type"), - ngx_string("ETag"), - ngx_string("From"), - ngx_string("Max-Forwards"), - ngx_string("Referer"), - ngx_string("Proxy-Authorization"), - ngx_string("User-Agent"), - ngx_string(""), + static ngx_http_js_header_t headers_in[] = { +#define header(name, fl) { ngx_string(name), fl, 0 } + header("Content-Type", NJS_HEADER_SINGLE), + header("ETag", NJS_HEADER_SINGLE), + header("From", NJS_HEADER_SINGLE), + header("Max-Forwards", NJS_HEADER_SINGLE), + header("Referer", NJS_HEADER_SINGLE), + header("Proxy-Authorization", NJS_HEADER_SINGLE), + header("User-Agent", NJS_HEADER_SINGLE), + header("Cookie", NJS_HEADER_SEMICOLON), + header("", 0), +#undef header }; r = JS_GetOpaque(obj, NGX_QJS_CLASS_ID_HTTP_HEADERS_IN); @@ -8577,11 +8499,11 @@ ngx_http_qjs_headers_in_own_property(JSContext *cx, JSPropertyDescriptor *pdesc, flags = 0; - for (h = single_headers_in; h->len > 0; h++) { - if (h->len == name.len - && ngx_strncasecmp(h->data, name.data, name.len) == 0) + for (h = headers_in; h->name.len > 0; h++) { + if (h->name.len == name.len + && ngx_strncasecmp(h->name.data, name.data, name.len) == 0) { - flags |= NJS_HEADER_SINGLE; + flags = h->flags; break; } } @@ -8670,7 +8592,7 @@ ngx_http_qjs_headers_out_handler(JSContext *cx, ngx_http_request_t *r, ngx_table_elt_t *header, *h, **ph; if (flags & NJS_HEADER_GET) { - return ngx_http_qjs_header_generic(cx, r, &r->headers_out.headers, NULL, + return ngx_http_qjs_header_generic(cx, r, &r->headers_out.headers, name, pdesc, flags); } diff --git a/nginx/t/js_headers.t b/nginx/t/js_headers.t index 7cf91099..ea8a31c6 100644 --- a/nginx/t/js_headers.t +++ b/nginx/t/js_headers.t @@ -496,7 +496,7 @@ $t->write_file('test.js', <try_run('no njs')->plan(51); +$t->try_run('no njs')->plan(53); ############################################################################### @@ -600,6 +600,19 @@ like(http( . 'Host: localhost' . CRLF . CRLF ), qr/foo: bar1,\s?bar2/, 'r.headersIn duplicate generic'); +like(http( + 'GET /hdr_in HTTP/1.0' . CRLF + . 'Proxy-Connection: keep-alive' . CRLF + . 'Host: localhost' . CRLF . CRLF +), qr/proxy-connection: keep-alive/, 'r.headersIn no slot header'); + +like(http( + 'GET /hdr_in HTTP/1.0' . CRLF + . 'Proxy-Connection: foo1' . CRLF + . 'Proxy-Connection: foo2' . CRLF + . 'Host: localhost' . CRLF . CRLF +), qr/proxy-connection: foo1,\s?foo2/, 'r.headersIn no slot header duplicate'); + like(http_get('/in_lowkey'), qr/X{16}/, 'r.headersIn name is not overwritten'); like(http(