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.

diff --git a/nginx/ngx_http_js_module.c b/nginx/ngx_http_js_module.c
index 3f38f86f79e3f816982d954c6b9ed9007aa9fd93..c73171b8222e75af7121ce18d4aa605e33094a5b 100644 (file)
--- 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 0678c3e6105c2cb8a40a087768c649387034d943..d2bb781c51b5dda75c7f306609a6124a88e01733 100644 (file)
--- 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 0e217e905f4011a911287ffbdd415cbd1b73959c..032bd4151e23d17577f08b907759103c36e0bcb6 100644 (file)
--- 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 d043bd5d9d2af6a46348937f08f04e741b683178..7130a0c228fdbf53e64f3e6b4eecfd01821494cc 100644 (file)
--- 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 cb44aa1f66b4b09404850f5245e018b377cba7b3..5ccc08a2a49d11c590e96ee5f27b2540623e01a1 100644 (file)
--- 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;
     }