Skip to content

Commit 241dffa

Browse files
xeioexxeioex
committed
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.
1 parent 02d8358 commit 241dffa

2 files changed

Lines changed: 110 additions & 175 deletions

File tree

‎nginx/ngx_http_js_module.c‎

Lines changed: 96 additions & 174 deletions
Original file line numberDiff line numberDiff line change
@@ -389,8 +389,8 @@ static njs_int_t ngx_http_js_header_out_special(njs_vm_t *vm,
389389
ngx_http_request_t *r, njs_str_t *v, njs_value_t *setval,
390390
njs_value_t *retval, ngx_table_elt_t **hh);
391391
static njs_int_t ngx_http_js_header_generic(njs_vm_t *vm,
392-
ngx_http_request_t *r, ngx_list_t *headers, ngx_table_elt_t **ph,
393-
unsigned flags, njs_str_t *name, njs_value_t *retval);
392+
ngx_http_request_t *r, ngx_list_t *headers, unsigned flags,
393+
njs_str_t *name, njs_value_t *retval);
394394
#endif
395395
static njs_int_t ngx_http_js_content_encoding(njs_vm_t *vm,
396396
ngx_http_request_t *r, unsigned flags, njs_str_t *name,
@@ -4393,20 +4393,24 @@ static njs_int_t
43934393
ngx_http_js_ext_header_in(njs_vm_t *vm, njs_object_prop_t *prop, uint32_t atom_id,
43944394
njs_value_t *value, njs_value_t *unused, njs_value_t *retval)
43954395
{
4396-
unsigned flags;
4397-
njs_int_t rc;
4398-
njs_str_t name, *h;
4399-
ngx_http_request_t *r;
4396+
unsigned flags;
4397+
njs_int_t rc;
4398+
njs_str_t name;
4399+
ngx_http_request_t *r;
4400+
ngx_http_js_header_t *h;
44004401

4401-
static njs_str_t single_headers_in[] = {
4402-
njs_str("Content-Type"),
4403-
njs_str("ETag"),
4404-
njs_str("From"),
4405-
njs_str("Max-Forwards"),
4406-
njs_str("Referer"),
4407-
njs_str("Proxy-Authorization"),
4408-
njs_str("User-Agent"),
4409-
njs_str(""),
4402+
static ngx_http_js_header_t headers_in[] = {
4403+
#define header(name, fl) { njs_str(name), fl, 0 }
4404+
header("Content-Type", NJS_HEADER_SINGLE),
4405+
header("ETag", NJS_HEADER_SINGLE),
4406+
header("From", NJS_HEADER_SINGLE),
4407+
header("Max-Forwards", NJS_HEADER_SINGLE),
4408+
header("Referer", NJS_HEADER_SINGLE),
4409+
header("Proxy-Authorization", NJS_HEADER_SINGLE),
4410+
header("User-Agent", NJS_HEADER_SINGLE),
4411+
header("Cookie", NJS_HEADER_SEMICOLON),
4412+
header("", 0),
4413+
#undef header
44104414
};
44114415

44124416
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
44294433

44304434
flags = 0;
44314435

4432-
for (h = single_headers_in; h->length > 0; h++) {
4433-
if (h->length == name.length
4434-
&& ngx_strncasecmp(h->start, name.start, name.length) == 0)
4436+
for (h = headers_in; h->name.len > 0; h++) {
4437+
if (h->name.len == name.length
4438+
&& ngx_strncasecmp(h->name.data, name.start, name.length) == 0)
44354439
{
4436-
flags |= NJS_HEADER_SINGLE;
4440+
flags = h->flags;
44374441
break;
44384442
}
44394443
}
@@ -5142,48 +5146,11 @@ static njs_int_t
51425146
ngx_http_js_header_in(njs_vm_t *vm, ngx_http_request_t *r, unsigned flags,
51435147
njs_str_t *name, njs_value_t *retval)
51445148
{
5145-
u_char *lowcase_key;
5146-
ngx_uint_t hash;
5147-
ngx_table_elt_t **ph;
5148-
ngx_http_header_t *hh;
5149-
ngx_http_core_main_conf_t *cmcf;
5150-
u_char storage[128];
5151-
51525149
if (retval == NULL) {
51535150
return NJS_OK;
51545151
}
51555152

5156-
/* look up hashed headers */
5157-
5158-
if (name->length < sizeof(storage)) {
5159-
lowcase_key = storage;
5160-
5161-
} else {
5162-
lowcase_key = ngx_pnalloc(r->pool, name->length);
5163-
if (lowcase_key == NULL) {
5164-
njs_vm_memory_error(vm);
5165-
return NJS_ERROR;
5166-
}
5167-
}
5168-
5169-
hash = ngx_hash_strlow(lowcase_key, name->start, name->length);
5170-
5171-
cmcf = ngx_http_get_module_main_conf(r, ngx_http_core_module);
5172-
5173-
hh = ngx_hash_find(&cmcf->headers_in_hash, hash, lowcase_key,
5174-
name->length);
5175-
5176-
ph = NULL;
5177-
5178-
if (hh) {
5179-
if (hh->offset == offsetof(ngx_http_headers_in_t, cookie)) {
5180-
flags |= NJS_HEADER_SEMICOLON;
5181-
}
5182-
5183-
ph = (ngx_table_elt_t **) ((char *) &r->headers_in + hh->offset);
5184-
}
5185-
5186-
return ngx_http_js_header_generic(vm, r, &r->headers_in.headers, ph, flags,
5153+
return ngx_http_js_header_generic(vm, r, &r->headers_in.headers, flags,
51875154
name, retval);
51885155
}
51895156

@@ -5203,9 +5170,8 @@ ngx_http_js_header_out(njs_vm_t *vm, ngx_http_request_t *r, unsigned flags,
52035170
njs_opaque_value_t lvalue;
52045171

52055172
if (retval != NULL && setval == NULL) {
5206-
return ngx_http_js_header_generic(vm, r, &r->headers_out.headers, NULL,
5207-
flags, name, retval);
5208-
5173+
return ngx_http_js_header_generic(vm, r, &r->headers_out.headers, flags,
5174+
name, retval);
52095175
}
52105176

52115177
part = &r->headers_out.headers.part;
@@ -5430,52 +5396,46 @@ ngx_http_js_header_out_special(njs_vm_t *vm, ngx_http_request_t *r,
54305396

54315397
static njs_int_t
54325398
ngx_http_js_header_generic(njs_vm_t *vm, ngx_http_request_t *r,
5433-
ngx_list_t *headers, ngx_table_elt_t **ph, unsigned flags, njs_str_t *name,
5434-
njs_value_t *retval)
5399+
ngx_list_t *headers, unsigned flags, njs_str_t *name, njs_value_t *retval)
54355400
{
54365401
u_char sep;
54375402
njs_chb_t chain;
54385403
njs_int_t rc, ret;
54395404
ngx_uint_t i;
54405405
njs_value_t *value;
54415406
ngx_list_part_t *part;
5442-
ngx_table_elt_t *header, *h;
5443-
5444-
if (ph == NULL) {
5445-
/* iterate over all headers */
5446-
5447-
ph = &header;
5448-
part = &headers->part;
5449-
h = part->elts;
5407+
ngx_table_elt_t *header, *h, **ph;
54505408

5451-
for (i = 0; /* void */ ; i++) {
5409+
ph = &header;
5410+
part = &headers->part;
5411+
h = part->elts;
54525412

5453-
if (i >= part->nelts) {
5454-
if (part->next == NULL) {
5455-
break;
5456-
}
5413+
for (i = 0; /* void */ ; i++) {
54575414

5458-
part = part->next;
5459-
h = part->elts;
5460-
i = 0;
5415+
if (i >= part->nelts) {
5416+
if (part->next == NULL) {
5417+
break;
54615418
}
54625419

5463-
if (h[i].hash == 0
5464-
|| name->length != h[i].key.len
5465-
|| ngx_strncasecmp(name->start, h[i].key.data, name->length)
5466-
!= 0)
5467-
{
5468-
continue;
5469-
}
5420+
part = part->next;
5421+
h = part->elts;
5422+
i = 0;
5423+
}
54705424

5471-
*ph = &h[i];
5472-
ph = &h[i].next;
5425+
if (h[i].hash == 0
5426+
|| name->length != h[i].key.len
5427+
|| ngx_strncasecmp(name->start, h[i].key.data, name->length) != 0)
5428+
{
5429+
continue;
54735430
}
54745431

5475-
*ph = NULL;
5476-
ph = &header;
5432+
*ph = &h[i];
5433+
ph = &h[i].next;
54775434
}
54785435

5436+
*ph = NULL;
5437+
ph = &header;
5438+
54795439
if (*ph == NULL) {
54805440
njs_value_undefined_set(retval);
54815441
return NJS_DECLINED;
@@ -8375,51 +8335,46 @@ ngx_http_qjs_headers_in_own_property_names(JSContext *cx,
83758335

83768336
static njs_int_t
83778337
ngx_http_qjs_header_generic(JSContext *cx, ngx_http_request_t *r,
8378-
ngx_list_t *headers, ngx_table_elt_t **ph, ngx_str_t *name,
8379-
JSPropertyDescriptor *pdesc, unsigned flags)
8338+
ngx_list_t *headers, ngx_str_t *name, JSPropertyDescriptor *pdesc,
8339+
unsigned flags)
83808340
{
83818341
u_char sep;
83828342
JSValue val;
83838343
njs_chb_t chain;
83848344
ngx_uint_t i;
83858345
ngx_list_part_t *part;
8386-
ngx_table_elt_t *header, *h;
8387-
8388-
if (ph == NULL) {
8389-
/* iterate over all headers */
8390-
8391-
ph = &header;
8392-
part = &headers->part;
8393-
h = part->elts;
8346+
ngx_table_elt_t *header, *h, **ph;
83948347

8395-
for (i = 0; /* void */ ; i++) {
8348+
ph = &header;
8349+
part = &headers->part;
8350+
h = part->elts;
83968351

8397-
if (i >= part->nelts) {
8398-
if (part->next == NULL) {
8399-
break;
8400-
}
8352+
for (i = 0; /* void */ ; i++) {
84018353

8402-
part = part->next;
8403-
h = part->elts;
8404-
i = 0;
8354+
if (i >= part->nelts) {
8355+
if (part->next == NULL) {
8356+
break;
84058357
}
84068358

8407-
if (h[i].hash == 0
8408-
|| name->len != h[i].key.len
8409-
|| ngx_strncasecmp(name->data, h[i].key.data, name->len)
8410-
!= 0)
8411-
{
8412-
continue;
8413-
}
8359+
part = part->next;
8360+
h = part->elts;
8361+
i = 0;
8362+
}
84148363

8415-
*ph = &h[i];
8416-
ph = &h[i].next;
8364+
if (h[i].hash == 0
8365+
|| name->len != h[i].key.len
8366+
|| ngx_strncasecmp(name->data, h[i].key.data, name->len) != 0)
8367+
{
8368+
continue;
84178369
}
84188370

8419-
*ph = NULL;
8420-
ph = &header;
8371+
*ph = &h[i];
8372+
ph = &h[i].next;
84218373
}
84228374

8375+
*ph = NULL;
8376+
ph = &header;
8377+
84238378
if (*ph == NULL) {
84248379
return 0;
84258380
}
@@ -8500,44 +8455,7 @@ static int
85008455
ngx_http_qjs_header_in(JSContext *cx, ngx_http_request_t *r, unsigned flags,
85018456
ngx_str_t *name, JSPropertyDescriptor *pdesc)
85028457
{
8503-
u_char *lowcase_key;
8504-
ngx_uint_t hash;
8505-
ngx_table_elt_t **ph;
8506-
ngx_http_header_t *hh;
8507-
ngx_http_core_main_conf_t *cmcf;
8508-
u_char storage[128];
8509-
8510-
/* look up hashed headers */
8511-
8512-
if (name->len < sizeof(storage)) {
8513-
lowcase_key = storage;
8514-
8515-
} else {
8516-
lowcase_key = ngx_pnalloc(r->pool, name->len);
8517-
if (lowcase_key == NULL) {
8518-
(void) JS_ThrowOutOfMemory(cx);
8519-
return -1;
8520-
}
8521-
}
8522-
8523-
hash = ngx_hash_strlow(lowcase_key, name->data, name->len);
8524-
8525-
cmcf = ngx_http_get_module_main_conf(r, ngx_http_core_module);
8526-
8527-
hh = ngx_hash_find(&cmcf->headers_in_hash, hash, lowcase_key,
8528-
name->len);
8529-
8530-
ph = NULL;
8531-
8532-
if (hh) {
8533-
if (hh->offset == offsetof(ngx_http_headers_in_t, cookie)) {
8534-
flags |= NJS_HEADER_SEMICOLON;
8535-
}
8536-
8537-
ph = (ngx_table_elt_t **) ((char *) &r->headers_in + hh->offset);
8538-
}
8539-
8540-
return ngx_http_qjs_header_generic(cx, r, &r->headers_in.headers, ph, name,
8458+
return ngx_http_qjs_header_generic(cx, r, &r->headers_in.headers, name,
85418459
pdesc, flags);
85428460
}
85438461

@@ -8546,20 +8464,24 @@ static int
85468464
ngx_http_qjs_headers_in_own_property(JSContext *cx, JSPropertyDescriptor *pdesc,
85478465
JSValueConst obj, JSAtom prop)
85488466
{
8549-
int ret;
8550-
unsigned flags;
8551-
ngx_str_t name, *h;
8552-
ngx_http_request_t *r;
8467+
int ret;
8468+
unsigned flags;
8469+
ngx_str_t name;
8470+
ngx_http_request_t *r;
8471+
ngx_http_js_header_t *h;
85538472

8554-
static ngx_str_t single_headers_in[] = {
8555-
ngx_string("Content-Type"),
8556-
ngx_string("ETag"),
8557-
ngx_string("From"),
8558-
ngx_string("Max-Forwards"),
8559-
ngx_string("Referer"),
8560-
ngx_string("Proxy-Authorization"),
8561-
ngx_string("User-Agent"),
8562-
ngx_string(""),
8473+
static ngx_http_js_header_t headers_in[] = {
8474+
#define header(name, fl) { ngx_string(name), fl, 0 }
8475+
header("Content-Type", NJS_HEADER_SINGLE),
8476+
header("ETag", NJS_HEADER_SINGLE),
8477+
header("From", NJS_HEADER_SINGLE),
8478+
header("Max-Forwards", NJS_HEADER_SINGLE),
8479+
header("Referer", NJS_HEADER_SINGLE),
8480+
header("Proxy-Authorization", NJS_HEADER_SINGLE),
8481+
header("User-Agent", NJS_HEADER_SINGLE),
8482+
header("Cookie", NJS_HEADER_SEMICOLON),
8483+
header("", 0),
8484+
#undef header
85638485
};
85648486

85658487
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,
85778499

85788500
flags = 0;
85798501

8580-
for (h = single_headers_in; h->len > 0; h++) {
8581-
if (h->len == name.len
8582-
&& ngx_strncasecmp(h->data, name.data, name.len) == 0)
8502+
for (h = headers_in; h->name.len > 0; h++) {
8503+
if (h->name.len == name.len
8504+
&& ngx_strncasecmp(h->name.data, name.data, name.len) == 0)
85838505
{
8584-
flags |= NJS_HEADER_SINGLE;
8506+
flags = h->flags;
85858507
break;
85868508
}
85878509
}
@@ -8670,7 +8592,7 @@ ngx_http_qjs_headers_out_handler(JSContext *cx, ngx_http_request_t *r,
86708592
ngx_table_elt_t *header, *h, **ph;
86718593

86728594
if (flags & NJS_HEADER_GET) {
8673-
return ngx_http_qjs_header_generic(cx, r, &r->headers_out.headers, NULL,
8595+
return ngx_http_qjs_header_generic(cx, r, &r->headers_out.headers,
86748596
name, pdesc, flags);
86758597
}
86768598

0 commit comments

Comments
 (0)