ArrayBuffer: fix copy-on-write for external data

Previously, writes to ArrayBuffers returned by nginx body accessors
or raw exportKey() could modify the request body or key material.

Now, njs_vm_value_array_buffer_set() takes a shared argument, like
other creation APIs.  Shared data uses copy-on-write; private
WebCrypto results and joined fetch response bodies remain writable
without copying.

The issue was introduced in f6485009 (0.5.1).

diff --git a/external/njs_webcrypto_module.c b/external/njs_webcrypto_module.c
index 8ed5fbbdde30d6bbda594d484ed39189c4aea8d9..c93a2298d4c56dc45afa2c0642b7ee39d426d6e5 100644 (file)
--- a/external/njs_webcrypto_module.c
+++ b/external/njs_webcrypto_module.c
@@ -933,7 +933,7 @@ njs_cipher_pkey(njs_vm_t *vm, njs_str_t *data, njs_webcrypto_key_t *key,
         goto fail;
     }
 
-    ret = njs_vm_value_array_buffer_set(vm, retval, dst, outlen);
+    ret = njs_vm_value_array_buffer_set(vm, retval, dst, outlen, 0);
 
 fail:
 
@@ -1120,7 +1120,7 @@ njs_cipher_aes_gcm(njs_vm_t *vm, njs_str_t *data, njs_webcrypto_key_t *key,
         outlen += taglen;
     }
 
-    ret = njs_vm_value_array_buffer_set(vm, retval, dst, outlen);
+    ret = njs_vm_value_array_buffer_set(vm, retval, dst, outlen, 0);
 
 fail:
 
@@ -1416,7 +1416,7 @@ njs_cipher_aes_ctr(njs_vm_t *vm, njs_str_t *data, njs_webcrypto_key_t *key,
 
 done:
 
-    ret = njs_vm_value_array_buffer_set(vm, retval, dst, len);
+    ret = njs_vm_value_array_buffer_set(vm, retval, dst, len, 0);
 
 fail:
 
@@ -1535,7 +1535,7 @@ njs_cipher_aes_cbc(njs_vm_t *vm, njs_str_t *data, njs_webcrypto_key_t *key,
 
     olen += olen2;
 
-    ret = njs_vm_value_array_buffer_set(vm, retval, dst, olen);
+    ret = njs_vm_value_array_buffer_set(vm, retval, dst, olen, 0);
 
 fail:
 
@@ -1635,7 +1635,7 @@ njs_cipher_aes_kw(njs_vm_t *vm, njs_str_t *data, njs_webcrypto_key_t *key,
         goto fail;
     }
 
-    ret = njs_vm_value_array_buffer_set(vm, retval, dst, olen);
+    ret = njs_vm_value_array_buffer_set(vm, retval, dst, olen, 0);
 
 fail:
 
@@ -1815,7 +1815,7 @@ njs_ext_derive_ecdh(njs_vm_t *vm, njs_value_t *args, njs_uint_t nargs,
                                      dkey, 0);
     } else {
         ret = njs_vm_value_array_buffer_set(vm, njs_value_arg(&lvalue), k,
-                                            length);
+                                            length, 0);
     }
 
     if (njs_slow_path(ret != NJS_OK)) {
@@ -2127,7 +2127,7 @@ free:
                                      dkey, 0);
     } else {
         ret = njs_vm_value_array_buffer_set(vm, njs_value_arg(&lvalue), k,
-                                            length);
+                                            length, 0);
     }
 
     if (njs_slow_path(ret != NJS_OK)) {
@@ -2179,7 +2179,7 @@ njs_ext_digest(njs_vm_t *vm, njs_value_t *args, njs_uint_t nargs,
         goto fail;
     }
 
-    ret = njs_vm_value_array_buffer_set(vm, njs_value_arg(&result), dst, olen);
+    ret = njs_vm_value_array_buffer_set(vm, njs_value_arg(&result), dst, olen, 0);
     if (njs_slow_path(ret != NJS_OK)) {
         goto fail;
     }
@@ -2490,7 +2490,7 @@ njs_export_raw_ec(njs_vm_t *vm, njs_webcrypto_key_t *key, njs_value_t *retval)
         return NJS_ERROR;
     }
 
-    return njs_vm_value_array_buffer_set(vm, retval, dst, size);
+    return njs_vm_value_array_buffer_set(vm, retval, dst, size, 0);
 }
 
 
@@ -4920,7 +4920,7 @@ njs_ext_sign(njs_vm_t *vm, njs_value_t *args, njs_uint_t nargs,
 
     if (!verify) {
         ret = njs_vm_value_array_buffer_set(vm, njs_value_arg(&result), dst,
-                                            outlen);
+                                            outlen, 0);
         if (njs_slow_path(ret != NJS_OK)) {
             goto fail;
         }
@@ -5084,7 +5084,7 @@ njs_webcrypto_export_key_raw(njs_vm_t *vm, njs_webcrypto_key_t *key,
 #endif
 
         return njs_vm_value_array_buffer_set(vm, retval, key->u.s.raw.start,
-                                             key->u.s.raw.length);
+                                             key->u.s.raw.length, 1);
     }
 
     return NJS_ERROR;
@@ -6054,7 +6054,7 @@ njs_webcrypto_array_buffer(njs_vm_t *vm, njs_value_t *retval,
 
     memcpy(dst, start, length);
 
-    return njs_vm_value_array_buffer_set(vm, retval, dst, length);
+    return njs_vm_value_array_buffer_set(vm, retval, dst, length, 0);
 }
 
 

diff --git a/nginx/ngx_http_js_module.c b/nginx/ngx_http_js_module.c
index 898e9db4541d5e7012e3ca7d31c3182e073f6844..d36796c069d8595f5510775756a00c219581194b 100644 (file)
--- a/nginx/ngx_http_js_module.c
+++ b/nginx/ngx_http_js_module.c
@@ -3680,7 +3680,7 @@ ngx_http_js_body_to_value(njs_vm_t *vm, ngx_http_js_ctx_t *ctx,
     case NGX_JS_BODY_ARRAY_BUFFER:
         return njs_vm_value_array_buffer_set(vm, retval,
                                              ctx->body_read_data,
-                                             ctx->body_read_len);
+                                             ctx->body_read_len, 1);
 
     case NGX_JS_BODY_JSON:
         ret = njs_vm_value_string_create(vm, njs_value_arg(&arg),

diff --git a/nginx/ngx_js_fetch.c b/nginx/ngx_js_fetch.c
index 9af568214a698d811ebf8b1c40288763edbe26f5..003ded35d6b44904711cd2e5ff3eece1e65df0ac 100644 (file)
--- a/nginx/ngx_js_fetch.c
+++ b/nginx/ngx_js_fetch.c
@@ -2033,7 +2033,7 @@ ngx_request_js_ext_body(njs_vm_t *vm, njs_value_t *args, njs_uint_t nargs,
     case NGX_JS_BODY_ARRAY_BUFFER:
         ret = njs_vm_value_array_buffer_set(vm, njs_value_arg(&result),
                                             request->body.data,
-                                            request->body.len);
+                                            request->body.len, 1);
         if (ret != NJS_OK) {
             njs_vm_memory_error(vm);
             return NJS_ERROR;
@@ -2196,7 +2196,7 @@ ngx_response_js_ext_body(njs_vm_t *vm, njs_value_t *args,
     switch (type) {
     case NGX_JS_BODY_ARRAY_BUFFER:
         ret = njs_vm_value_array_buffer_set(vm, njs_value_arg(&result),
-                                            string.start, string.length);
+                                            string.start, string.length, 0);
         if (ret != NJS_OK) {
             njs_vm_memory_error(vm);
             return NJS_ERROR;

diff --git a/src/njs.h b/src/njs.h
index c726b244af259e69063bf50993df6e0661566ab8..54908c359c13ee638974161903c71912be17f89e 100644 (file)
--- a/src/njs.h
+++ b/src/njs.h
@@ -432,8 +432,13 @@ NJS_EXPORT njs_int_t njs_vm_value_string_create_chb(njs_vm_t *vm,
 NJS_EXPORT njs_int_t njs_vm_string_compare(njs_vm_t *vm, const njs_value_t *v1,
     const njs_value_t *v2);
 
+/*
+ * Sets an ArrayBuffer value without copying data or taking ownership.
+ * Shared data is copied on the first write; otherwise it must be writable.
+ * The caller must keep the data alive while the buffer refers to it.
+ */
 NJS_EXPORT njs_int_t njs_vm_value_array_buffer_set(njs_vm_t *vm,
-    njs_value_t *value, const u_char *start, uint32_t size);
+    njs_value_t *value, const u_char *start, uint32_t size, njs_bool_t shared);
 
 NJS_EXPORT njs_int_t njs_value_buffer_get(njs_vm_t *vm, njs_value_t *value,
     njs_str_t *dst);

diff --git a/src/njs_vm.c b/src/njs_vm.c
index ce9b0ef6762eec73c37842595a65688aa30627e4..6bd459f19cc7c6e8ee71364c0b5ea1067e5b3c0e 100644 (file)
--- a/src/njs_vm.c
+++ b/src/njs_vm.c
@@ -1011,7 +1011,7 @@ njs_value_string_get(njs_vm_t *vm, njs_value_t *value, njs_str_t *dst)
 
 njs_int_t
 njs_vm_value_array_buffer_set(njs_vm_t *vm, njs_value_t *value,
-    const u_char *start, uint32_t size)
+    const u_char *start, uint32_t size, njs_bool_t shared)
 {
     njs_array_buffer_t  *array;
 
@@ -1020,6 +1020,7 @@ njs_vm_value_array_buffer_set(njs_vm_t *vm, njs_value_t *value,
         return NJS_ERROR;
     }
 
+    array->object.shared = shared;
     array->u.data = (u_char *) start;
     array->size = size;
 

diff --git a/src/test/njs_externals_test.c b/src/test/njs_externals_test.c
index d34ec1f2078c1d7e907679013ec0762b173a2ded..22d5673c35531a0cfeeb5e4bbdd8ddfba2ac91db 100644 (file)
--- a/src/test/njs_externals_test.c
+++ b/src/test/njs_externals_test.c
@@ -230,6 +230,16 @@ njs_unit_test_r_buffer(njs_vm_t *vm, njs_object_prop_t *prop, uint32_t unused,
 }
 
 
+static njs_int_t
+njs_unit_test_r_array_buffer(njs_vm_t *vm, njs_object_prop_t *prop,
+    uint32_t unused, njs_value_t *value, njs_value_t *setval,
+    njs_value_t *retval)
+{
+    return njs_vm_value_array_buffer_set(vm, retval,
+                                         (u_char *) "АБВГДЕЁЖЗИЙ", 22, 1);
+}
+
+
 static njs_int_t
 njs_unit_test_r_vars(njs_vm_t *vm, njs_object_prop_t *self, uint32_t atom_id,
     njs_value_t *value, njs_value_t *setval, njs_value_t *retval)
@@ -850,6 +860,15 @@ static njs_external_t  njs_unit_test_r_external[] = {
         }
     },
 
+    {
+        .flags = NJS_EXTERN_PROPERTY,
+        .name.string = njs_str("arrayBuffer"),
+        .enumerable = 1,
+        .u.property = {
+            .handler = njs_unit_test_r_array_buffer,
+        }
+    },
+
     {
         .flags = NJS_EXTERN_METHOD,
         .name.string = njs_str("method"),

diff --git a/src/test/njs_unit_test.c b/src/test/njs_unit_test.c
index 1fa222511ff9940307c9482a293bba6f15338fd5..d47b2ee589516c01ac28081b56e6ab5e2af90cea 100644 (file)
--- a/src/test/njs_unit_test.c
+++ b/src/test/njs_unit_test.c
@@ -22039,6 +22039,11 @@ static njs_unit_test_t  njs_externals_test[] =
               "[b[0], $r.buffer[0]]"),
       njs_str("64,208") },
 
+    { njs_str("var b = new Uint8Array($r.arrayBuffer);"
+              "b[0] = '@'.codePointAt(0);"
+              "[b[0], new Uint8Array($r.arrayBuffer)[0]]"),
+      njs_str("64,208") },
+
     { njs_str("$r.buffer instanceof Buffer"),
       njs_str("true") },
 
@@ -23978,6 +23983,98 @@ njs_vm_object_alloc_test(njs_vm_t *vm, njs_opts_t *opts, njs_stat_t *stat)
 }
 
 
+static njs_int_t
+njs_vm_array_buffer_test(njs_vm_t *unused, njs_opts_t *opts, njs_stat_t *stat)
+{
+    u_char             *start, *data[2];
+    njs_vm_t           *vm;
+    njs_int_t           ret;
+    njs_uint_t          i;
+    njs_str_t           bytes;
+    njs_vm_opt_t        options;
+    njs_opaque_value_t  buffers[2], retval;
+
+    static const njs_str_t  names[] = {
+        njs_str("privateBuffer"), njs_str("sharedBuffer")
+    };
+
+    static const njs_str_t  script = njs_str(
+        "new Uint8Array(privateBuffer)[0] = 88;"
+        "new DataView(sharedBuffer).setUint8(0, 88);");
+
+    njs_vm_opt_init(&options);
+    options.init = 1;
+
+    vm = njs_vm_create(&options);
+    if (vm == NULL) {
+        return NJS_ERROR;
+    }
+
+    ret = NJS_ERROR;
+
+    for (i = 0; i < njs_nitems(buffers); i++) {
+        data[i] = njs_mp_alloc(njs_vm_memory_pool(vm), 2);
+        if (data[i] == NULL) {
+            ret = NJS_ERROR;
+            goto done;
+        }
+
+        memcpy(data[i], "ab", 2);
+
+        ret = njs_vm_value_array_buffer_set(vm, njs_value_arg(&buffers[i]),
+                                            data[i], 2, i);
+        if (ret != NJS_OK) {
+            goto done;
+        }
+
+        ret = njs_vm_bind(vm, &names[i], njs_value_arg(&buffers[i]), 0);
+        if (ret != NJS_OK) {
+            goto done;
+        }
+    }
+
+    start = script.start;
+    ret = njs_vm_compile(vm, &start, start + script.length);
+    if (ret != NJS_OK) {
+        goto done;
+    }
+
+    ret = njs_vm_start(vm, njs_value_arg(&retval));
+    if (ret != NJS_OK) {
+        goto done;
+    }
+
+    for (i = 0; i < njs_nitems(buffers); i++) {
+        ret = njs_vm_value_to_bytes(vm, &bytes, njs_value_arg(&buffers[i]));
+        if (ret != NJS_OK) {
+            goto done;
+        }
+
+        if (bytes.length != 2 || bytes.start[0] != 88 || bytes.start[1] != 'b'
+            || (bytes.start != data[i]) != (i != 0)
+            || data[i][0] != ((i == 0) ? 88 : 'a'))
+        {
+            njs_printf("ArrayBuffer shared=%ui: unexpected backing data\n", i);
+            ret = NJS_ERROR;
+            goto done;
+        }
+    }
+
+done:
+
+    njs_vm_destroy(vm);
+
+    if (ret == NJS_OK) {
+        stat->passed++;
+
+    } else {
+        stat->failed++;
+    }
+
+    return NJS_OK;
+}
+
+
 static njs_int_t
 njs_chb_test(njs_vm_t *vm, njs_opts_t *opts, njs_stat_t *stat)
 {
@@ -24456,6 +24553,8 @@ njs_vm_internal_api_test(njs_unit_test_t unused[], size_t num, njs_str_t *name,
     } tests[] = {
         { njs_vm_object_alloc_test,
           njs_str("njs_vm_object_alloc_test") },
+        { njs_vm_array_buffer_test,
+          njs_str("njs_vm_array_buffer_test") },
         { njs_chb_test,
           njs_str("njs_chb_test") },
         { njs_sort_test,