details: https://hg.nginx.org/njs/rev/9578cc729205
branches:
changeset: 1802:9578cc729205
user: Dmitry Volyntsev <xeioex@nginx.com>
date: Wed Jan 12 17:59:42 2022 +0000
description:
Fixed Array.prototype.join() when array is changed while iterating.

Previously, the function used optimization for ordinary arrays with no
gaps (so called fast arrays). For a fast array code took elements
directly from internal flat C array. The direct pointer may become
invalid as side-effect of custom toString() method for an element.

Specifically, the pointer was passed directly to
njs_value_to_primitive() which attempts to call toString() followed by
valueOf(). When the array size is changed as a side-effect of
toString() and not a string value is returned by toString() the pointer
becomes invalid and is passed to valueOf() which causes use-after-free.

The fix is to eliminate the micro-optimization which uses direct pointers.

Found by PolyGlot fuzzing framework.

This closes #444 issue on Github.

diffstat:

src/njs_array.c | 34 +++++++++-------------------------
src/test/njs_unit_test.c | 8 ++++++++
2 files changed, 17 insertions(+), 25 deletions(-)

diffs (83 lines):

diff -r 2adc0d3fc2bd -r 9578cc729205 src/njs_array.c
--- a/src/njs_array.c Wed Jan 12 17:58:19 2022 +0000
+++ b/src/njs_array.c Wed Jan 12 17:59:42 2022 +0000
@@ -1573,7 +1573,6 @@ njs_array_prototype_join(njs_vm_t *vm, n
njs_int_t ret;
njs_chb_t chain;
njs_utf8_t utf8;
- njs_array_t *array;
njs_value_t *value, *this, entry;
njs_string_prop_t separator, string;

@@ -1606,18 +1605,11 @@ njs_array_prototype_join(njs_vm_t *vm, n
}

length = 0;
- array = NULL;
utf8 = njs_is_byte_string(&separator) ? NJS_STRING_BYTE : NJS_STRING_UTF8;

- if (njs_is_fast_array(this)) {
- array = njs_array(this);
- len = array->length;
-
- } else {
- ret = njs_object_length(vm, this, &len);
- if (njs_slow_path(ret == NJS_ERROR)) {
- return ret;
- }
+ ret = njs_object_length(vm, this, &len);
+ if (njs_slow_path(ret == NJS_ERROR)) {
+ return ret;
}

if (njs_slow_path(len == 0)) {
@@ -1625,25 +1617,17 @@ njs_array_prototype_join(njs_vm_t *vm, n
return NJS_OK;
}

+ value = &entry;
+
njs_chb_init(&chain, vm->mem_pool);

for (i = 0; i < len; i++) {
- if (njs_fast_path(array != NULL
- && array->object.fast_array
- && njs_is_valid(&array->start[i])))
- {
- value = &array->start[i];
-
- } else {
- ret = njs_value_property_i64(vm, this, i, &entry);
- if (njs_slow_path(ret == NJS_ERROR)) {
- return ret;
- }
-
- value = &entry;
+ ret = njs_value_property_i64(vm, this, i, value);
+ if (njs_slow_path(ret == NJS_ERROR)) {
+ return ret;
}

- if (njs_is_valid(value) && !njs_is_null_or_undefined(value)) {
+ if (!njs_is_null_or_undefined(value)) {
if (!njs_is_string(value)) {
last = njs_chb_current(&chain);

diff -r 2adc0d3fc2bd -r 9578cc729205 src/test/njs_unit_test.c
--- a/src/test/njs_unit_test.c Wed Jan 12 17:58:19 2022 +0000
+++ b/src/test/njs_unit_test.c Wed Jan 12 17:59:42 2022 +0000
@@ -4801,6 +4801,14 @@ static njs_unit_test_t njs_test[] =
".map(v=>v.join(''))"),
njs_str(",1345,,1,13,13,13") },

+ { njs_str("var o = { toString: () => {"
+ " for (var i = 0; i < 0x10; i++) {a.push(1)};"
+ " return {};"
+ "}};"
+ "var a = [o];"
+ "a.join()"),
+ njs_str("TypeError: Cannot convert object to primitive value") },
+
{ njs_str("Array.prototype.splice.call({0:0,1:1,2:2,3:3,length:4},0,3,4,5)"),
njs_str("0,1,2") },

_______________________________________________
nginx-devel mailing list -- nginx-devel@nginx.org
To unsubscribe send an email to nginx-devel-leave@nginx.org
%(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s