From 8fef12ae007c9868670fb71c9995706a908e8c7b Mon Sep 17 00:00:00 2001 From: Dmitry Volyntsev Date: Thu, 29 Oct 2026 18:53:23 -0700 Subject: [PATCH] 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). --- external/njs_webcrypto_module.c | 24 ++++---- nginx/ngx_http_js_module.c | 2 +- nginx/ngx_js_fetch.c | 4 +- src/njs.h | 7 ++- src/njs_vm.c | 3 +- src/test/njs_externals_test.c | 19 +++++++ src/test/njs_unit_test.c | 99 +++++++++++++++++++++++++++++++++ 7 files changed, 141 insertions(+), 17 deletions(-) diff --git a/external/njs_webcrypto_module.c b/external/njs_webcrypto_module.c index 8ed5fbbd..c93a2298 100644 --- 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 898e9db4..d36796c0 100644 --- 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 9af56821..003ded35 100644 --- 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 c726b244..54908c35 100644 --- 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 ce9b0ef6..6bd459f1 100644 --- 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 d34ec1f2..22d5673c 100644 --- 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 1fa22251..d47b2ee5 100644 --- 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, -- 2.47.3