details:
https://github.com/nginx/njs/commit/4eeb4c527c8031b676c720c7ebb49ad356233a31
branches: master
commit: 4eeb4c527c8031b676c720c7ebb49ad356233a31
user: Dmitry Volyntsev <xei...@nginx.com>
date: Thu, 28 Aug 2025 15:20:31 -0700
description:
QuickJS: fixed potential heap-use-after-free.
Previously in QuickJS engine, fields allocated from memory pool linked
to QuickJS engine lifetime were stored in nginx data structs.
This causes a heap-use-after-free if QuickJS engine is destroyed
earlier than a last access from nginx. For example, it becomes
visible when moving NJS cleanup handler from pool->cleanup to
r->cleanup.
The fix is to only store references in nginx objects allocated from
nginx memory pool.
---
nginx/ngx_http_js_module.c | 46 ++++++++++++++------------------------------
nginx/ngx_js.c | 7 ++++---
nginx/ngx_js.h | 3 ++-
nginx/ngx_qjs_fetch.c | 26 +++++++++++++++----------
nginx/ngx_stream_js_module.c | 2 +-
5 files changed, 37 insertions(+), 47 deletions(-)
diff --git a/nginx/ngx_http_js_module.c b/nginx/ngx_http_js_module.c
index 3f38f86f..c73171b8 100644
--- a/nginx/ngx_http_js_module.c
+++ b/nginx/ngx_http_js_module.c
@@ -5173,7 +5173,7 @@ ngx_http_qjs_ext_internal_redirect(JSContext *cx,
JSValueConst this_val,
"internalRedirect cannot be called while filtering");
}
- if (ngx_qjs_string(cx, argv[0], &ctx->redirect_uri) != NGX_OK) {
+ if (ngx_qjs_string(cx, r->pool, argv[0], &ctx->redirect_uri) != NGX_OK) {
return JS_EXCEPTION;
}
@@ -5452,7 +5452,7 @@ ngx_http_qjs_ext_return(JSContext *cx, JSValueConst
this_val,
ctx = ngx_http_get_module_ctx(r, ngx_http_js_module);
if (status < NGX_HTTP_BAD_REQUEST || !JS_IsNullOrUndefined(argv[1])) {
- if (ngx_qjs_string(cx, argv[1], &body) != NGX_OK) {
+ if (ngx_qjs_string(cx, r->pool, argv[1], &body) != NGX_OK) {
return JS_ThrowOutOfMemory(cx);
}
@@ -5557,7 +5557,7 @@ ngx_http_qjs_ext_send(JSContext *cx, JSValueConst
this_val,
ll = &out;
for (n = 0; n < (ngx_uint_t) argc; n++) {
- if (ngx_qjs_string(cx, argv[n], &s) != NGX_OK) {
+ if (ngx_qjs_string(cx, r->pool, argv[n], &s) != NGX_OK) {
return JS_ThrowTypeError(cx, "failed to convert arg");
}
@@ -5905,7 +5905,7 @@ ngx_http_qjs_ext_subrequest(JSContext *cx, JSValueConst
this_val,
"the primary request");
}
- if (ngx_qjs_string(cx, argv[0], &uri) != NGX_OK) {
+ if (ngx_qjs_string(cx, r->pool, argv[0], &uri) != NGX_OK) {
return JS_ThrowTypeError(cx, "failed to convert uri arg");
}
@@ -5931,7 +5931,7 @@ ngx_http_qjs_ext_subrequest(JSContext *cx, JSValueConst
this_val,
arg = argv[1];
if (JS_IsString(arg)) {
- if (ngx_qjs_string(cx, arg, &args) != NGX_OK) {
+ if (ngx_qjs_string(cx, r->pool, arg, &args) != NGX_OK) {
return JS_ThrowTypeError(cx, "failed to convert args");
}
@@ -5952,7 +5952,7 @@ ngx_http_qjs_ext_subrequest(JSContext *cx, JSValueConst
this_val,
}
if (!JS_IsUndefined(value)) {
- rc = ngx_qjs_string(cx, value, &args);
+ rc = ngx_qjs_string(cx, r->pool, value, &args);
JS_FreeValue(cx, value);
if (rc != NGX_OK) {
@@ -5976,7 +5976,7 @@ ngx_http_qjs_ext_subrequest(JSContext *cx, JSValueConst
this_val,
}
if (!JS_IsUndefined(value)) {
- rc = ngx_qjs_string(cx, value, &method_name);
+ rc = ngx_qjs_string(cx, r->pool, value, &method_name);
JS_FreeValue(cx, value);
if (rc != NGX_OK) {
@@ -6003,7 +6003,7 @@ ngx_http_qjs_ext_subrequest(JSContext *cx, JSValueConst
this_val,
}
if (!JS_IsUndefined(value)) {
- rc = ngx_qjs_string(cx, value, &body_arg);
+ rc = ngx_qjs_string(cx, r->pool, value, &body_arg);
JS_FreeValue(cx, value);
if (rc != NGX_OK) {
@@ -6397,7 +6397,7 @@ ngx_http_qjs_variables_set_property(JSContext *cx,
JSValueConst obj,
return -1;
}
- if (ngx_qjs_string(cx, value, &s) != NGX_OK) {
+ if (ngx_qjs_string(cx, r->pool, value, &s) != NGX_OK) {
return -1;
}
@@ -6876,7 +6876,7 @@ ngx_http_qjs_headers_out_handler(JSContext *cx,
ngx_http_request_t *r,
}
}
- rc = ngx_qjs_string(cx, v, &s);
+ rc = ngx_qjs_string(cx, r->pool, v, &s);
if (qjs_is_array(cx, *value)) {
JS_FreeValue(cx, v);
@@ -6908,16 +6908,7 @@ ngx_http_qjs_headers_out_handler(JSContext *cx,
ngx_http_request_t *r,
h->key.data = p;
h->key.len = name->len;
- p = ngx_pnalloc(r->pool, s.len);
- if (p == NULL) {
- h->hash = 0;
- (void) JS_ThrowOutOfMemory(cx);
- return -1;
- }
-
- ngx_memcpy(p, s.data, s.len);
-
- h->value.data = p;
+ h->value.data = s.data;
h->value.len = s.len;
h->hash = 1;
@@ -6977,7 +6968,7 @@ ngx_http_qjs_headers_out_special_handler(JSContext *cx,
ngx_http_request_t *r,
setval = JS_UNDEFINED;
}
- rc = ngx_qjs_string(cx, setval, &s);
+ rc = ngx_qjs_string(cx, r->pool, setval, &s);
if (value != NULL && qjs_is_array(cx, *value)) {
JS_FreeValue(cx, setval);
@@ -7046,16 +7037,7 @@ done:
}
if (h != NULL) {
- p = ngx_pnalloc(r->pool, s.len);
- if (p == NULL) {
- h->hash = 0;
- (void) JS_ThrowOutOfMemory(cx);
- return -1;
- }
-
- ngx_memcpy(p, s.data, s.len);
-
- h->value.data = p;
+ h->value.data = s.data;
h->value.len = s.len;
h->hash = 1;
}
@@ -7212,7 +7194,7 @@ ngx_http_qjs_headers_out_content_type(JSContext *cx,
ngx_http_request_t *r,
setval = *value;
}
- rc = ngx_qjs_string(cx, setval, &s);
+ rc = ngx_qjs_string(cx, r->pool, setval, &s);
if (qjs_is_array(cx, *value)) {
JS_FreeValue(cx, setval);
diff --git a/nginx/ngx_js.c b/nginx/ngx_js.c
index 0678c3e6..d2bb781c 100644
--- a/nginx/ngx_js.c
+++ b/nginx/ngx_js.c
@@ -1462,7 +1462,8 @@ ngx_qjs_integer(JSContext *cx, JSValueConst val,
ngx_int_t *n)
ngx_int_t
-ngx_qjs_string(JSContext *cx, JSValueConst val, ngx_str_t *dst)
+ngx_qjs_string(JSContext *cx, ngx_pool_t *pool, JSValueConst val,
+ ngx_str_t *dst)
{
size_t len, byte_offset, byte_length;
u_char *start;
@@ -1496,7 +1497,7 @@ ngx_qjs_string(JSContext *cx, JSValueConst val, ngx_str_t
*dst)
start += byte_offset;
dst->len = byte_length;
- dst->data = njs_mp_alloc(e->pool, dst->len);
+ dst->data = ngx_pnalloc(pool, dst->len);
if (dst->data == NULL) {
return NGX_ERROR;
}
@@ -1513,7 +1514,7 @@ string:
return NGX_ERROR;
}
- start = njs_mp_alloc(e->pool, len);
+ start = ngx_pnalloc(pool, len);
if (start == NULL) {
JS_FreeCString(cx, str);
return NGX_ERROR;
diff --git a/nginx/ngx_js.h b/nginx/ngx_js.h
index 0e217e90..032bd415 100644
--- a/nginx/ngx_js.h
+++ b/nginx/ngx_js.h
@@ -354,7 +354,8 @@ ngx_int_t ngx_qjs_call(JSContext *cx, JSValue function,
JSValue *argv,
int argc);
ngx_int_t ngx_qjs_exception(ngx_engine_t *e, ngx_str_t *s);
ngx_int_t ngx_qjs_integer(JSContext *cx, JSValueConst val, ngx_int_t *n);
-ngx_int_t ngx_qjs_string(JSContext *cx, JSValueConst val, ngx_str_t *str);
+ngx_int_t ngx_qjs_string(JSContext *cx, ngx_pool_t *pool, JSValueConst val,
+ ngx_str_t *dst);
JSValue ngx_qjs_ext_fetch(JSContext *cx, JSValueConst this_val, int argc,
JSValueConst *argv);
diff --git a/nginx/ngx_qjs_fetch.c b/nginx/ngx_qjs_fetch.c
index d043bd5d..7130a0c2 100644
--- a/nginx/ngx_qjs_fetch.c
+++ b/nginx/ngx_qjs_fetch.c
@@ -622,7 +622,7 @@ ngx_qjs_request_ctor(JSContext *cx, ngx_js_request_t
*request,
}
if (JS_IsString(input)) {
- rc = ngx_qjs_string(cx, input, &request->url);
+ rc = ngx_qjs_string(cx, pool, input, &request->url);
if (rc != NGX_OK) {
JS_ThrowInternalError(cx, "failed to convert url arg");
return NGX_ERROR;
@@ -694,7 +694,7 @@ ngx_qjs_request_ctor(JSContext *cx, ngx_js_request_t
*request,
}
if (!JS_IsUndefined(value)) {
- rc = ngx_qjs_string(cx, value, &request->method);
+ rc = ngx_qjs_string(cx, pool, value, &request->method);
JS_FreeValue(cx, value);
if (rc != NGX_OK) {
@@ -774,7 +774,7 @@ ngx_qjs_request_ctor(JSContext *cx, ngx_js_request_t
*request,
}
if (!JS_IsUndefined(value)) {
- if (ngx_qjs_string(cx, value, &request->body) != NGX_OK) {
+ if (ngx_qjs_string(cx, pool, value, &request->body) != NGX_OK) {
JS_FreeValue(cx, value);
JS_ThrowInternalError(cx, "invalid Request body");
return NGX_ERROR;
@@ -866,7 +866,7 @@ ngx_qjs_fetch_response_ctor(JSContext *cx, JSValueConst
new_target, int argc,
}
if (!JS_IsUndefined(value)) {
- ret = ngx_qjs_string(cx, value, &response->status_text);
+ ret = ngx_qjs_string(cx, pool, value, &response->status_text);
JS_FreeValue(cx, value);
if (ret < 0) {
@@ -913,7 +913,7 @@ ngx_qjs_fetch_response_ctor(JSContext *cx, JSValueConst
new_target, int argc,
body = argv[0];
if (!JS_IsNullOrUndefined(body)) {
- if (ngx_qjs_string(cx, body, &bd) != NGX_OK) {
+ if (ngx_qjs_string(cx, pool, body, &bd) != NGX_OK) {
return JS_ThrowInternalError(cx, "invalid Response body");
}
@@ -1054,16 +1054,19 @@ static ngx_int_t
ngx_qjs_headers_fill_header_free(JSContext *cx, ngx_js_headers_t *headers,
JSValue prop_name, JSValue prop_value)
{
- ngx_int_t rc;
- ngx_str_t name, value;
+ ngx_int_t rc;
+ ngx_str_t name, value;
+ ngx_pool_t *pool;
+
+ pool = ngx_qjs_external_pool(cx, JS_GetContextOpaque(cx));
- if (ngx_qjs_string(cx, prop_name, &name) != NGX_OK) {
+ if (ngx_qjs_string(cx, pool, prop_name, &name) != NGX_OK) {
JS_FreeValue(cx, prop_name);
JS_FreeValue(cx, prop_value);
return NGX_ERROR;
}
- if (ngx_qjs_string(cx, prop_value, &value) != NGX_OK) {
+ if (ngx_qjs_string(cx, pool, prop_value, &value) != NGX_OK) {
JS_FreeValue(cx, prop_name);
JS_FreeValue(cx, prop_value);
return NGX_ERROR;
@@ -1950,6 +1953,7 @@ ngx_qjs_ext_fetch_headers_set(JSContext *cx, JSValueConst
this_val,
ngx_int_t rc;
ngx_str_t name, value;
ngx_uint_t i;
+ ngx_pool_t *pool;
ngx_list_part_t *part;
ngx_js_tb_elt_t *h, **ph, **pp;
ngx_js_headers_t *headers;
@@ -1965,7 +1969,9 @@ ngx_qjs_ext_fetch_headers_set(JSContext *cx, JSValueConst
this_val,
return JS_EXCEPTION;
}
- rc = ngx_qjs_string(cx, argv[1], &value);
+ pool = ngx_qjs_external_pool(cx, JS_GetContextOpaque(cx));
+
+ rc = ngx_qjs_string(cx, pool, argv[1], &value);
if (rc != NGX_OK) {
JS_FreeCString(cx, (const char *) name.data);
return JS_EXCEPTION;
diff --git a/nginx/ngx_stream_js_module.c b/nginx/ngx_stream_js_module.c
index cb44aa1f..5ccc08a2 100644
--- a/nginx/ngx_stream_js_module.c
+++ b/nginx/ngx_stream_js_module.c
@@ -2651,7 +2651,7 @@ ngx_stream_qjs_variables_set_property(JSContext *cx,
JSValueConst obj,
return -1;
}
- if (ngx_qjs_string(cx, value, &val) != NGX_OK) {
+ if (ngx_qjs_string(cx, s->connection->pool, value, &val) != NGX_OK) {
return -1;
}
_______________________________________________
nginx-devel mailing list
nginx-devel@nginx.org
https://mailman.nginx.org/mailman/listinfo/nginx-devel
