RegExp: fix Symbol.replace() with an inconsistent match object

Previously, the matched string returned by exec() was assumed to be a
substring of the subject at the reported index.
njs_string_get_substitution() converted "index + matched length" to a byte
offset without a bounds check, and the replace loop advanced the end of the
last match by the byte size of the matched string instead of its character
length.  As a result the $' expansion read outside the subject, and on a
multibyte subject the result could start in the middle of a UTF-8 sequence.

The fix is to pass the head and tail slices of the subject to
njs_string_get_substitution() and to track the end of the last match as a
character index.  The function no longer converts indexes to offsets, so the
out of range conversion is prevented by construction.

This is an extended fix for #1116 pull request on Github.

Reported by Basavaraj S m (basavaraj@digiscrypt.com).

diff --git a/src/njs_regexp.c b/src/njs_regexp.c
index 6ab21123937b576d15702d99ca7ff6dbbf3e5e8f..e902dc7b3607870ae4648aeb9b88584202a209d8 100644 (file)
--- a/src/njs_regexp.c
+++ b/src/njs_regexp.c
@@ -1263,9 +1263,9 @@ njs_int_t
 njs_regexp_prototype_symbol_replace(njs_vm_t *vm, njs_value_t *args,
     njs_uint_t nargs, njs_index_t unused, njs_value_t *retval)
 {
-    int64_t            n, last_index, ncaptures, pos, length;
-    const u_char       *p, *next;
-    njs_str_t          rep, m;
+    int64_t            n, last_index, ncaptures, pos, length, next, mlength;
+    const u_char       *p, *start;
+    njs_str_t          rep, m, head, tail;
     njs_int_t          ret;
     njs_arr_t          results;
     njs_chb_t          chain;
@@ -1275,7 +1275,7 @@ njs_regexp_prototype_symbol_replace(njs_vm_t *vm, njs_value_t *args,
     njs_value_t        *arguments, *r, *rx, *string, *replace;
     njs_value_t        s_lvalue, r_lvalue, value, matched, groups;
     njs_function_t     *func_replace;
-    njs_string_prop_t  s;
+    njs_string_prop_t  s, ms;
 
     rx = njs_argument(args, 0);
 
@@ -1390,8 +1390,7 @@ njs_regexp_prototype_symbol_replace(njs_vm_t *vm, njs_value_t *args,
     }
 
     i = 0;
-    p = s.start;
-    next = p;
+    next = 0;
 
     while (i < results.items) {
         r = njs_arr_item(&results, i++);
@@ -1422,6 +1421,10 @@ njs_regexp_prototype_symbol_replace(njs_vm_t *vm, njs_value_t *args,
 
         pos = njs_max(njs_min(pos, (int64_t) length), 0);
 
+        mlength = njs_string_prop(vm, &ms, &matched);
+
+        p = njs_string_offset(&s, pos);
+
         ret = njs_object_length(vm, r, &ncaptures);
         if (njs_slow_path(ret != NJS_OK)) {
             goto exception;
@@ -1466,9 +1469,19 @@ njs_regexp_prototype_symbol_replace(njs_vm_t *vm, njs_value_t *args,
                 }
             }
 
-            ret = njs_string_get_substitution(vm, &matched, string, pos,
-                                              arguments, ncaptures, &groups,
-                                              replace, retval);
+            m.start = ms.start;
+            m.length = ms.size;
+
+            head.start = s.start;
+            head.length = p - s.start;
+
+            start = njs_string_offset(&s, njs_min(pos + mlength, length));
+            tail.start = (u_char *) start;
+            tail.length = (s.start + s.size) - start;
+
+            ret = njs_string_get_substitution(vm, &m, &head, &tail, arguments,
+                                              ncaptures, &groups, replace,
+                                              retval);
 
         } else {
             ret = njs_array_expand(vm, array, 0,
@@ -1501,17 +1514,14 @@ njs_regexp_prototype_symbol_replace(njs_vm_t *vm, njs_value_t *args,
             goto exception;
         }
 
-        p = njs_string_offset(&s, pos);
-
-        if (p >= next) {
-            njs_chb_append(&chain, next, p - next);
+        if (pos >= next) {
+            start = njs_string_offset(&s, next);
+            njs_chb_append(&chain, start, p - start);
 
             njs_string_get(vm, retval, &rep);
             njs_chb_append_str(&chain, &rep);
 
-            njs_string_get(vm, &matched, &m);
-
-            next = p + m.length;
+            next = pos + mlength;
         }
 
         if (!func_replace && njs_object_slots(r)) {
@@ -1524,8 +1534,9 @@ njs_regexp_prototype_symbol_replace(njs_vm_t *vm, njs_value_t *args,
         }
     }
 
-    if (next < s.start + s.size) {
-        njs_chb_append(&chain, next, s.start + s.size - next);
+    if (next < length) {
+        start = njs_string_offset(&s, next);
+        njs_chb_append(&chain, start, s.start + s.size - start);
     }
 
     ret = njs_string_create_chb(vm, retval, &chain);

diff --git a/src/njs_string.c b/src/njs_string.c
index 125ec967d693e90fdba2526ecce9a4aa7b0c7c3e..272ac4e2de5a68be17f2001a0499afd8cadc2763 100644 (file)
--- a/src/njs_string.c
+++ b/src/njs_string.c
@@ -2941,18 +2941,16 @@ done:
 
 
 njs_int_t
-njs_string_get_substitution(njs_vm_t *vm, njs_value_t *matched,
-    njs_value_t *string, int64_t pos, njs_value_t *captures, int64_t ncaptures,
+njs_string_get_substitution(njs_vm_t *vm, njs_str_t *matched, njs_str_t *head,
+    njs_str_t *tail, njs_value_t *captures, int64_t ncaptures,
     njs_value_t *groups, njs_value_t *replacement, njs_value_t *retval)
 {
-    u_char             c, c2, *p, *r, *end;
-    size_t             length;
-    int64_t            tail, n;
-    njs_str_t          rep, str, cap;
-    njs_int_t          ret;
-    njs_chb_t          chain;
-    njs_value_t        name, value;
-    njs_string_prop_t  s, m;
+    u_char       c, c2, *p, *r, *end;
+    int64_t      n;
+    njs_str_t    rep, str, cap;
+    njs_int_t    ret;
+    njs_chb_t    chain;
+    njs_value_t  name, value;
 
     njs_string_get(vm, replacement, &rep);
     p = rep.start;
@@ -2984,26 +2982,17 @@ njs_string_get_substitution(njs_vm_t *vm, njs_value_t *matched,
             break;
 
         case '&':
-            (void) njs_string_prop(vm, &m, matched);
-            njs_chb_append(&chain, m.start, m.size);
+            njs_chb_append_str(&chain, matched);
             p += 2;
             break;
 
         case '`':
-            (void) njs_string_prop(vm, &s, string);
-            n = njs_string_offset(&s, pos) - s.start;
-            njs_chb_append(&chain, s.start, n);
+            njs_chb_append_str(&chain, head);
             p += 2;
             break;
 
         case '\'':
-            length = njs_string_prop(vm, &m, matched);
-            (void) njs_string_prop(vm, &s, string);
-
-            tail = njs_string_offset(&s, pos + length) - s.start;
-
-            njs_chb_append(&chain, &s.start[tail],
-                           njs_max((int64_t) s.size - tail, 0));
+            njs_chb_append_str(&chain, tail);
             p += 2;
             break;
 
@@ -3104,7 +3093,7 @@ njs_string_prototype_replace(njs_vm_t *vm, njs_value_t *args, njs_uint_t nargs,
     size_t             length, size, end_of_last_match;
     int64_t            pos;
     njs_int_t          ret;
-    njs_str_t          str;
+    njs_str_t          str, m, head, tail;
     njs_chb_t          chain;
     njs_value_t        *this, *search, *replace;
     njs_value_t        search_lvalue, replace_lvalue, replacer, value,
@@ -3198,9 +3187,20 @@ njs_string_prototype_replace(njs_vm_t *vm, njs_value_t *args, njs_uint_t nargs,
         return NJS_OK;
     }
 
+    m.start = s.start;
+    m.length = s.size;
+
     if (!replaceAll) {
+        end = njs_string_offset(&string, pos);
+
         if (func_replace == NULL) {
-            ret = njs_string_get_substitution(vm, search, this, pos, NULL, 0,
+            head.start = string.start;
+            head.length = end - string.start;
+
+            tail.start = (u_char *) end + s.size;
+            tail.length = (string.start + string.size) - (end + s.size);
+
+            ret = njs_string_get_substitution(vm, &m, &head, &tail, NULL, 0,
                                               NULL, replace, &value);
             if (njs_slow_path(ret != NJS_OK)) {
                 return ret;
@@ -3223,8 +3223,6 @@ njs_string_prototype_replace(njs_vm_t *vm, njs_value_t *args, njs_uint_t nargs,
             }
         }
 
-        end = njs_string_offset(&string, pos);
-
         (void) njs_string_prop(vm, &ret_string, &value);
 
         size = string.size + ret_string.size - s.size;
@@ -3247,8 +3245,16 @@ njs_string_prototype_replace(njs_vm_t *vm, njs_value_t *args, njs_uint_t nargs,
     start = string.start;
 
     do {
+        end = njs_string_offset(&string, pos);
+
         if (func_replace == NULL) {
-            ret = njs_string_get_substitution(vm, search, this, pos, NULL, 0,
+            head.start = string.start;
+            head.length = end - string.start;
+
+            tail.start = (u_char *) end + s.size;
+            tail.length = (string.start + string.size) - (end + s.size);
+
+            ret = njs_string_get_substitution(vm, &m, &head, &tail, NULL, 0,
                                               NULL, replace, &value);
             if (njs_slow_path(ret != NJS_OK)) {
                 return ret;
@@ -3271,7 +3277,6 @@ njs_string_prototype_replace(njs_vm_t *vm, njs_value_t *args, njs_uint_t nargs,
             }
         }
 
-        end = njs_string_offset(&string, pos);
         (void) njs_string_prop(vm, &ret_string, &value);
 
         njs_chb_append(&chain, start, end - start);

diff --git a/src/njs_string.h b/src/njs_string.h
index ac3fd5e16115ca95a26a38708ad78ea948751389..33722c6f552dd2764078ef62a509be9959cd4205 100644 (file)
--- a/src/njs_string.h
+++ b/src/njs_string.h
@@ -174,8 +174,8 @@ njs_int_t njs_string_atob(njs_vm_t *vm, njs_value_t *args, njs_uint_t nargs,
 
 njs_int_t njs_string_prototype_concat(njs_vm_t *vm, njs_value_t *args,
     njs_uint_t nargs, njs_index_t unused, njs_value_t *retval);
-njs_int_t njs_string_get_substitution(njs_vm_t *vm, njs_value_t *matched,
-    njs_value_t *string, int64_t pos, njs_value_t *captures, int64_t ncaptures,
+njs_int_t njs_string_get_substitution(njs_vm_t *vm, njs_str_t *matched,
+    njs_str_t *head, njs_str_t *tail, njs_value_t *captures, int64_t ncaptures,
     njs_value_t *groups, njs_value_t *replacement, njs_value_t *retval);
 
 

diff --git a/src/test/njs_unit_test.c b/src/test/njs_unit_test.c
index 547eeef928ef073cc14d4e122a2af99afff82bb1..8ffd3a97b67ee9f628503566eb7f75b1ce6dca3b 100644 (file)
--- a/src/test/njs_unit_test.c
+++ b/src/test/njs_unit_test.c
@@ -10138,6 +10138,57 @@ static njs_unit_test_t  njs_test[] =
               "'abc'.replaceAll(re, '@$1|$2|$3|$4|$99|$100|@')"),
       njs_str("@|X||Y|Z|0|@") },
 
+    { njs_str("var n = 0, s = 'αβγ';"
+              "var re = {global: false, flags: '', exec: () =>"
+              "  n++ ? null : {0: 'y'.repeat(100), index: 0}};"
+              "RegExp.prototype[Symbol.replace].call(re, s, \"[$']\")"),
+      njs_str("[]") },
+
+    { njs_str("var n = 0, s = 'αβγδε';"
+              "var re = {global: false, flags: '', exec: () =>"
+              "  n++ ? null : {0: 'y'.repeat(1000), index: 2}};"
+              "RegExp.prototype[Symbol.replace].call(re, s, \"<$'>\")"),
+      njs_str("αβ<>") },
+
+    { njs_str("var n = 0, s = 'αβγ';"
+              "var re = {global: false, flags: '', exec: () =>"
+              "  n++ ? null : {0: 'y', index: 0}};"
+              "RegExp.prototype[Symbol.replace].call(re, s, '')"),
+      njs_str("βγ") },
+
+    { njs_str("var n = 0, s = 'αβγ';"
+              "var re = {global: false, flags: '', exec: () =>"
+              "  n++ ? null : {0: 'y', index: 1e9}};"
+              "RegExp.prototype[Symbol.replace].call(re, s, '[$`]')"),
+      njs_str("αβγ[αβγ]") },
+
+    { njs_str("var n = 0, s = 'αβγδ';"
+              "var re = {global: false, flags: '', exec: () =>"
+              "  n++ ? null : {0: 'αβ', index: 1}};"
+              "RegExp.prototype[Symbol.replace].call(re, s, \"[$&|$`|$']\")"),
+      njs_str("α[αβ|α|δ]δ") },
+
+    { njs_str("var n = 0, s = 'αβγδεζ';"
+              "var a = [{0: 'y', index: 5}, {0: 'y', index: 0}];"
+              "var re = {global: true, flags: 'g',"
+              "          exec: () => n < a.length ? a[n++] : null};"
+              "RegExp.prototype[Symbol.replace].call(re, s, '<$&>')"),
+      njs_str("αβγδε<y>") },
+
+    { njs_str("var n = 0;"
+              "var a = [{0: 'yyyy', index: 0}, {0: 'z', index: 3}];"
+              "var re = {global: true, flags: 'g',"
+              "          exec: () => n < a.length ? a[n++] : null};"
+              "RegExp.prototype[Symbol.replace].call(re, 'abc', '[1]')"),
+      njs_str("[1]") },
+
+    { njs_str("var n = 0;"
+              "var re = {global: false, flags: '', exec: () =>"
+              "  n++ ? null : {0: 'yyyyyyyyy', index: 1}};"
+              "RegExp.prototype[Symbol.replace].call(re, 'abcdef',"
+              "                                      \"[$&|$`|$']\")"),
+      njs_str("a[yyyyyyyyy|a|]") },
+
     { njs_str("var a = [];"
               "Object.defineProperty(a, 32768, {});"
               "var re = /any_regexp/;"