Fixed heap-buffer-overflow in String.prototype.lastIndexOf().

Previously, the issue occurred when the searchValue is shorter in character length than "this" string, but longer in byte length.
nginx · May 28, 2021 · dc4232e · dc4232e
1 parent 72ff5c2
commit dc4232e
Show file tree

Hide file tree

Showing 2 changed files with 73 additions and 69 deletions.
diff --git a/src/njs_string.c b/src/njs_string.c
@@ -2230,114 +2230,94 @@ njs_string_prototype_last_index_of(njs_vm_t *vm, njs_value_t *args,
     njs_uint_t nargs, njs_index_t unused)
 {
     double             pos;
-    ssize_t            index, start, length, search_length;
+    int64_t            index, start, length, search_length;
     njs_int_t          ret;
-    njs_value_t        *value, *search_string, lvalue;
+    njs_value_t        *this, *search, search_lvalue;
     const u_char       *p, *end;
-    njs_string_prop_t  string, search;
-
-    ret = njs_string_object_validate(vm, njs_arg(args, nargs, 0));
-    if (njs_slow_path(ret != NJS_OK)) {
-        return ret;
-    }
-
-    index = -1;
+    njs_string_prop_t  string, s;
 
-    length = njs_string_prop(&string, njs_argument(args, 0));
+    this = njs_argument(args, 0);
 
-    search_string = njs_lvalue_arg(&lvalue, args, nargs, 1);
+    if (njs_slow_path(njs_is_null_or_undefined(this))) {
+        njs_type_error(vm, "cannot convert \"%s\"to object",
+                       njs_type_string(this->type));
+        return NJS_ERROR;
+    }
 
-    if (njs_slow_path(!njs_is_string(search_string))) {
-        ret = njs_value_to_string(vm, search_string, search_string);
-        if (njs_slow_path(ret != NJS_OK)) {
-            return ret;
-        }
+    ret = njs_value_to_string(vm, this, this);
+    if (njs_slow_path(ret != NJS_OK)) {
+        return NJS_ERROR;
     }
 
-    search_length = njs_string_prop(&search, search_string);
+    search = njs_lvalue_arg(&search_lvalue, args, nargs, 1);
+    ret = njs_value_to_string(vm, search, search);
+    if (njs_slow_path(ret != NJS_OK)) {
+        return ret;
+    }
 
-    if (length < search_length) {
-        goto done;
+    ret = njs_value_to_number(vm, njs_arg(args, nargs, 2), &pos);
+    if (njs_slow_path(ret != NJS_OK)) {
+        return ret;
     }
 
-    value = njs_arg(args, nargs, 2);
-
-    if (njs_slow_path(!njs_is_number(value))) {
-        ret = njs_value_to_number(vm, value, &pos);
-        if (njs_slow_path(ret != NJS_OK)) {
-            return ret;
-        }
+    if (!isnan(pos)) {
+        start = njs_number_to_integer(pos);
 
     } else {
-        pos = njs_number(value);
+        start = INT64_MAX;
     }
 
-    if (isnan(pos)) {
-        index = NJS_STRING_MAX_LENGTH;
+    length = njs_string_prop(&string, this);
 
-    } else {
-        index = njs_number_to_integer(pos);
+    start = njs_min(njs_max(start, 0), length);
 
-        if (index < 0) {
-            index = 0;
-        }
-    }
+    search_length = njs_string_prop(&s, search);
 
-    if (search_length == 0) {
-        index = njs_min(index, length);
-        goto done;
-    }
+    index = length - search_length;
 
-    if (index >= length) {
-        index = length - 1;
+    if (index > start) {
+        index = start;
     }
 
+    end = string.start + string.size;
+
     if (string.size == (size_t) length) {
         /* Byte or ASCII string. */
 
-        start = length - search.size;
+        p = &string.start[index];
 
-        if (index > start) {
-            index = start;
+        if (p > end - s.size) {
+            p = end - s.size;
         }
 
-        p = string.start + index;
-
-        do {
-            if (memcmp(p, search.start, search.size) == 0) {
+        for (; p >= string.start; p--) {
+            if (memcmp(p, s.start, s.size) == 0) {
+                index = p - string.start;
                 goto done;
             }
+        }
 
-            index--;
-            p--;
-
-        } while (p >= string.start);
+        index = -1;
 
     } else {
         /* UTF-8 string. */
 
-        end = string.start + string.size;
-        p = njs_string_offset(string.start, end, index);
-        end -= search.size;
-
-        while (p > end) {
-            index--;
-            p = njs_utf8_prev(p);
+        if (index < 0 || index == length) {
+            index = (search_length == 0) ? index : -1;
+            goto done;
         }
 
-        for ( ;; ) {
-            if (memcmp(p, search.start, search.size) == 0) {
+        p = njs_string_offset(string.start, end, index);
+
+        for (; p >= string.start; p = njs_utf8_prev(p)) {
+            if ((p + s.size) <= end && memcmp(p, s.start, s.size) == 0) {
                 goto done;
             }
 
             index--;
-
-            if (p <= string.start) {
-                break;
-            }
-
-            p = njs_utf8_prev(p);
         }
+
+        index = -1;
     }
 
 done:

diff --git a/src/test/njs_unit_test.c b/src/test/njs_unit_test.c
@@ -8008,7 +8008,28 @@ static njs_unit_test_t  njs_test[] =
     { njs_str("'a a'.toUTF8().indexOf('a', 1)"),
       njs_str("2") },
 
-    { njs_str("'abc'.lastIndexOf('abcdef')"),
+    { njs_str("'aaa'.lastIndexOf()"),
+      njs_str("-1") },
+
+    { njs_str("'aaa'.lastIndexOf('')"),
+      njs_str("3") },
+
+    { njs_str("'aaa'.lastIndexOf('a')"),
+      njs_str("2") },
+
+    { njs_str("'aaa'.lastIndexOf('aa')"),
+      njs_str("1") },
+
+    { njs_str("'aaa'.lastIndexOf('aaa')"),
+      njs_str("0") },
+
+    { njs_str("'aaa'.lastIndexOf('aaaa')"),
+      njs_str("-1") },
+
+    { njs_str("'a'.repeat(16).lastIndexOf(String.fromCodePoint(65533).repeat(15))"),
+      njs_str("-1") },
+
+    { njs_str("('α'+'a'.repeat(15)).lastIndexOf(String.fromCodePoint(65533).repeat(15))"),
       njs_str("-1") },
 
     { njs_str("'abc abc abc abc'.lastIndexOf('abc')"),
@@ -8077,6 +8098,9 @@ static njs_unit_test_t  njs_test[] =
     { njs_str("'β'.repeat(32).lastIndexOf('β')"),
       njs_str("31") },
 
+    { njs_str("'β'.repeat(32).lastIndexOf('β'.repeat(32))"),
+      njs_str("0") },
+
     { njs_str("'β'.repeat(32).lastIndexOf``"),
       njs_str("32") },