Mercurial > njs

changeset 2611:6569d5b04dd1

Find changesets by keywords (author, files, the commit message), revision number or hash, or revset expression.
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.
author Dmitry Volyntsev <xeioex@nginx.com>
date Thu, 28 Aug 2025 15:20:31 -0700
parents 54eab0317d4c
children 855a5a97651a
files nginx/ngx_http_js_module.c nginx/ngx_js.c nginx/ngx_js.h nginx/ngx_qjs_fetch.c nginx/ngx_stream_js_module.c
diffstat 5 files changed, 38 insertions(+), 48 deletions(-) [+]
line wrap: on
line diff
--- a/nginx/ngx_http_js_module.c	Thu Aug 28 16:29:22 2025 -0700
+++ b/nginx/ngx_http_js_module.c	Thu Aug 28 15:20:31 2025 -0700
@@ -5173,7 +5173,7 @@
                          "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 @@
     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 @@
     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 @@
                                      "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 @@
     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 @@
         }
 
         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 @@
         }
 
         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 @@
         }
 
         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 @@
         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 @@
             }
         }
 
-        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 @@
         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 @@
         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 @@
     }
 
     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 @@
         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);
--- a/nginx/ngx_js.c	Thu Aug 28 16:29:22 2025 -0700
+++ b/nginx/ngx_js.c	Thu Aug 28 15:20:31 2025 -0700
@@ -1462,7 +1462,8 @@
 
 
 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 @@
             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 @@
         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;
--- a/nginx/ngx_js.h	Thu Aug 28 16:29:22 2025 -0700
+++ b/nginx/ngx_js.h	Thu Aug 28 15:20:31 2025 -0700
@@ -354,7 +354,8 @@
     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);
--- a/nginx/ngx_qjs_fetch.c	Thu Aug 28 16:29:22 2025 -0700
+++ b/nginx/ngx_qjs_fetch.c	Thu Aug 28 15:20:31 2025 -0700
@@ -622,7 +622,7 @@
     }
 
     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 @@
         }
 
         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 @@
         }
 
         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 @@
         }
 
         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 @@
     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 @@
 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;
-
-    if (ngx_qjs_string(cx, prop_name, &name) != NGX_OK) {
+    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, 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_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 @@
         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;
--- a/nginx/ngx_stream_js_module.c	Thu Aug 28 16:29:22 2025 -0700
+++ b/nginx/ngx_stream_js_module.c	Thu Aug 28 15:20:31 2025 -0700
@@ -2651,7 +2651,7 @@
         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;
     }