CVE-2022-27008: Buffer Copy without Checking Size of Input ('Classic Buffer Overflow') (CWE-120)
value: nginx njs 0.7.2 is vulnerable to Buffer Overflow. Type confused in Array.prototype.concat() when a slow array appended element is fast array.
CWE description: The product copies an input buffer to an output buffer without verifying that the size of the input buffer is less than the size of the output buffer, leading to a buffer overflow.
CVSS v3 base score: 7.5
Published: 2022-04-14T15:15Z
Repository: https://github.com/nginx/njs
Language: C
Files changed: src/njs_array.c, src/test/njs_unit_test.c
Commit date: 2022-02-21 16:52:47 +0000
Commit message: Fixed Array.prototype.concat() when "this" is a slow array.

Previously, when the current appended element is fast array the "this"
array was expected to always be a fast array also.  This may not be the
case when the previous appended element was not fast thus converting
the "this" array to a slow form.

Previous fix introduced in 2c1382bab643 (0.7.2) was not complete, the
correct fix is to never assume "this" is fast, whereas njs_array_add()
may only be called with fast arrays.

This closes #471 issue in Github.

Diff: diff --git a/src/njs_array.c b/src/njs_array.c
index 7e81d6c6..81a7c155 100644
--- a/src/njs_array.c
+++ b/src/njs_array.c
@@ -362,10 +362,12 @@ njs_array_expand(njs_vm_t *vm, njs_array_t *array, uint32_t prepend,
 {
     uint32_t     free_before, free_after;
     uint64_t     size;
     njs_value_t  *start, *old;
 
+    njs_assert(array->object.fast_array);
+
     free_before = array->start - array->data;
     free_after = array->size - array->length - free_before;
 
     if (njs_fast_path(free_before >= prepend && free_after >= append)) {
         return NJS_OK;
@@ -1752,13 +1754,14 @@ njs_array_prototype_concat(njs_vm_t *vm, njs_value_t *args, njs_uint_t nargs,
                         }
 
                         njs_set_invalid(&retval);
                     }
 
-                    ret = njs_array_add(vm, array, &retval);
-                    if (njs_slow_path(ret != NJS_OK)) {
-                        return NJS_ERROR;
+                    ret = njs_value_property_i64_set(vm, &this, length,
+                                                     &retval);
+                    if (njs_slow_path(ret == NJS_ERROR)) {
+                        return ret;
                     }
                 }
 
                 continue;
             }
diff --git a/src/test/njs_unit_test.c b/src/test/njs_unit_test.c
index 89bc1615..dc18a13b 100644
--- a/src/test/njs_unit_test.c
+++ b/src/test/njs_unit_test.c
@@ -4251,10 +4251,15 @@ static njs_unit_test_t  njs_test[] =
     { njs_str(njs_declare_sparse_array("a", 64)
               "a[32] = 1; a = [1].concat(a);"
               "njs.dump([a[0], a[33],a.length])"),
       njs_str("[1,1,65]") },
 
+    { njs_str("var a = [1]; a[1111111] = 2;"
+              "var a2 = [3].concat(a, [4]);"
+              "njs.dump(a2)"),
+      njs_str("[3,1,<1111110 empty items>,2,4]") },
+
     { njs_str("var re = /abc/; re[Symbol.isConcatSpreadable] = true;"
               "re[0] = 1, re[1] = 2, re[2] = 3, re.length = 3;"
               "[].concat(re)"),
       njs_str("1,2,3") },
 

Vulnerable code:                     ret = njs_array_add(vm, array, &retval);
                    if (njs_slow_path(ret != NJS_OK)) {
                        return NJS_ERROR;
Fixed code:     njs_assert(array->object.fast_array);

                    ret = njs_value_property_i64_set(vm, &this, length,
                                                     &retval);
                    if (njs_slow_path(ret == NJS_ERROR)) {
                        return ret;
    { njs_str("var a = [1]; a[1111111] = 2;"
              "var a2 = [3].concat(a, [4]);"
              "njs.dump(a2)"),
      njs_str("[3,1,<1111110 empty items>,2,4]") },