Fix use-after-free in Array.prototype.sort()

Previously, njs_sort_indexed_properties() cached the fast array backing
store pointer (array->start) once and then iterates.  For a hole it
performed a generic property lookup that walks the prototype chain and
may invoke a user-defined getter.  A getter that grows the same array
triggers njs_array_expand(), which reallocates the backing store and
frees the old buffer, leaving the cached pointer dangling.  The
remaining iterations then read freed memory, and the stale values are
sorted and written back, exposing freed heap contents to script.

Reported by Sangsoo Jeong (78ResearchLab).

diff --git a/src/njs_array.c b/src/njs_array.c
index e91e6b22ef1f1ca4fb71ae36bf7bb122df42ae63..636dcfca9b5ba9a1e87453053eff79e035ec8571 100644 (file)
--- a/src/njs_array.c
+++ b/src/njs_array.c
@@ -2755,8 +2755,8 @@ njs_sort_indexed_properties(njs_vm_t *vm, njs_value_t *obj, int64_t length,
 {
     int64_t                i, ilength, nlen;
     njs_int_t              ret;
-    njs_array_t            *array, *keys;
-    njs_value_t            *start, *strings, key;
+    njs_array_t            *keys;
+    njs_value_t            *strings;
     njs_array_sort_ctx_t   ctx;
     njs_array_sort_slot_t  *p, *end, *slots, *newslots;
 
@@ -2771,9 +2771,6 @@ njs_sort_indexed_properties(njs_vm_t *vm, njs_value_t *obj, int64_t length,
     ctx.exception = 0;
 
     if (njs_fast_path(njs_is_fast_array(obj))) {
-        array = njs_array(obj);
-        start = array->start;
-
         slots = njs_mp_alloc(vm->mem_pool,
                              sizeof(njs_array_sort_slot_t) * length);
         if (njs_slow_path(slots == NULL)) {
@@ -2785,24 +2782,13 @@ njs_sort_indexed_properties(njs_vm_t *vm, njs_value_t *obj, int64_t length,
         p = slots;
 
         for (i = 0; i < length; i++) {
-            if (njs_fast_path(njs_is_valid(&start[i]))) {
-                /* not an empty value at index i. */
-                njs_value_assign(&p->value, &start[i]);
-
-            } else {
-                ret = njs_uint32_to_string(vm, &key, i);
-                if (njs_slow_path(ret != NJS_OK)) {
-                    goto exception;
-                }
-
-                ret = njs_value_property_val(vm, obj, &key, &p->value);
-                if (njs_slow_path(ret == NJS_ERROR)) {
-                    goto exception;
-                }
+            ret = njs_value_property_i64(vm, obj, i, &p->value);
+            if (njs_slow_path(ret == NJS_ERROR)) {
+                goto exception;
+            }
 
-                if (ret == NJS_DECLINED && skip_holes) {
-                    continue;
-                }
+            if (ret == NJS_DECLINED && skip_holes) {
+                continue;
             }
 
             if (njs_slow_path(njs_is_undefined(&p->value))) {

diff --git a/src/test/njs_unit_test.c b/src/test/njs_unit_test.c
index 26c894342066321e714ef1a4d61e3f5aac86e66a..7a28332c35d0c1ea98a6f89b86e261e0cc7e45c5 100644 (file)
--- a/src/test/njs_unit_test.c
+++ b/src/test/njs_unit_test.c
@@ -7970,6 +7970,16 @@ static njs_unit_test_t  njs_test[] =
               "njs.dump([undefined, 3, /*hole*/, 2, undefined, /*hole*/, 1].sort())"),
       njs_str("[1,2,3,4,undefined,undefined,<empty>]") },
 
+    /* A prototype getter for a hole reallocates the array being sorted. */
+
+    { njs_str("Object.defineProperty(Array.prototype, 1, {configurable: true,"
+              "  get() { for (var i = 0; i < 1024; i++) { this.push(i); }"
+              "          return 5; }});"
+              "var a = [3]; a[2] = 7; a.length = 3;"
+              "a.sort(function(x, y) { return x - y; });"
+              "a[0] === 3 && a[1] === 5 && a[2] === 7"),
+      njs_str("true") },
+
     { njs_str("var a = [3,2,1]; [a.toSorted(), a]"),
       njs_str("1,2,3,3,2,1") },