Fixed string offset map corruption in scope values hash.

The issue was introduced in e7caa46d (0.9.5).  When compile-time
UTF-8 string constants were copied into the values hash in
njs_scope_value_index(), the string data layout was calculated
incorrectly: the map offset did not account for the null terminator
added in e7caa46d, the "size" variable was overwritten corrupting
the subsequent memcpy, and the offset map was never initialized
to zero.

This caused SEGV/SIGBUS crashes for any multi-byte UTF-8 string
constant with more than 32 characters when accessing a character
at index >= 32 (e.g. via .replace() or bracket notation).  The bug
only manifested when the string byte size was 4-byte aligned, as
otherwise alignment padding absorbed the missing byte.

The fix factors out njs_string_data_size() and njs_string_data_init()
helpers shared by njs_string_alloc() and njs_scope_value_index(),
eliminating the duplicated layout logic that caused the divergence.

Found by Akshay Jain (akshaythe@gmail.com).

diff --git a/src/njs_scope.c b/src/njs_scope.c
index 9d7b5776d2e31c1cfe76044959806c52a6c9f3c2..398aeda974c0aad8389565b9aaf1d9cb45fdf44f 100644 (file)
--- a/src/njs_scope.c
+++ b/src/njs_scope.c
@@ -158,6 +158,9 @@ njs_scope_value_index(njs_vm_t *vm, const njs_value_t *src, njs_uint_t runtime,
     njs_object_prop_t    *pr;
     njs_flathsh_query_t  fhq;
 
+    /* Suppress "may be used uninitialized" warning. */
+    length = 0;
+
     is_string = 0;
     value_size = sizeof(njs_value_t);
 
@@ -194,12 +197,8 @@ njs_scope_value_index(njs_vm_t *vm, const njs_value_t *src, njs_uint_t runtime,
         if (is_string) {
             length = src->string.data->length;
 
-            if (size != length && length > NJS_STRING_MAP_STRIDE) {
-                size = njs_string_map_offset(size)
-                       + njs_string_map_size(length);
-            }
-
-            value_size += sizeof(njs_string_t) + size + 1;
+            value_size += sizeof(njs_string_t)
+                          + njs_string_data_size(size, length);
         }
 
         value_size += sizeof(njs_index_t);
@@ -217,11 +216,7 @@ njs_scope_value_index(njs_vm_t *vm, const njs_value_t *src, njs_uint_t runtime,
 
             value->string.data = string;
 
-            string->start = (u_char *) string + sizeof(njs_string_t);
-            string->length = src->string.data->length;
-            string->size = src->string.data->size;
-
-            string->start[size] = '\0';
+            njs_string_data_init(string, size, length);
 
             memcpy(string->start, start, size);
         }

diff --git a/src/njs_string.c b/src/njs_string.c
index 498da7e7de5994d12a33ae4cf01e6cf1c491f83b..ba781727c1c8c8bfa3f3818e32a5497965d0b7a8 100644 (file)
--- a/src/njs_string.c
+++ b/src/njs_string.c
@@ -147,12 +147,43 @@ njs_string_new(njs_vm_t *vm, njs_value_t *value, const u_char *start,
 }
 
 
+uint32_t
+njs_string_data_size(uint32_t size, uint32_t length)
+{
+    if (size != length && length > NJS_STRING_MAP_STRIDE) {
+        return njs_string_map_offset(size + njs_length("\0"))
+               + njs_string_map_size(length);
+    }
+
+    return size + njs_length("\0");
+}
+
+
+void
+njs_string_data_init(njs_string_t *string, uint32_t size, uint32_t length)
+{
+    uint32_t  map_offset, *map;
+
+    string->start = (u_char *) string + sizeof(njs_string_t);
+    string->size = size;
+    string->length = length;
+
+    string->start[size] = '\0';
+
+    if (size != length && length > NJS_STRING_MAP_STRIDE) {
+        map_offset = njs_string_map_offset(size + njs_length("\0"));
+        map = (uint32_t *) (string->start + map_offset);
+        map[0] = 0;
+    }
+}
+
+
 /* Underlying string data is zero-terminated. */
 u_char *
 njs_string_alloc(njs_vm_t *vm, njs_value_t *value, uint64_t size,
     uint64_t length)
 {
-    uint32_t      total, map_offset, *map;
+    uint32_t      total;
     njs_string_t  *string;
 
     if (njs_slow_path(size > NJS_STRING_MAX_LENGTH)) {
@@ -164,30 +195,14 @@ njs_string_alloc(njs_vm_t *vm, njs_value_t *value, uint64_t size,
     value->truth = size != 0;
     value->atom_id = NJS_ATOM_STRING_unknown;
 
-    if (size != length && length > NJS_STRING_MAP_STRIDE) {
-        map_offset = njs_string_map_offset(size + njs_length("\0"));
-        total = map_offset + njs_string_map_size(length);
-
-    } else {
-        map_offset = 0;
-        total = size + njs_length("\0");
-    }
+    total = njs_string_data_size(size, length);
 
     string = njs_mp_alloc(vm->mem_pool, sizeof(njs_string_t) + total);
 
     if (njs_fast_path(string != NULL)) {
         value->string.data = string;
 
-        string->start = (u_char *) string + sizeof(njs_string_t);
-        string->size = size;
-        string->length = length;
-
-        string->start[size] = '\0';
-
-        if (map_offset != 0) {
-            map = (uint32_t *) (string->start + map_offset);
-            map[0] = 0;
-        }
+        njs_string_data_init(string, size, length);
 
         return string->start;
     }

diff --git a/src/njs_string.h b/src/njs_string.h
index 2a676ee0c4b1e2f8c3d5ad7a12a04f499f45d6b7..ac3fd5e16115ca95a26a38708ad78ea948751389 100644 (file)
--- a/src/njs_string.h
+++ b/src/njs_string.h
@@ -77,6 +77,11 @@ struct njs_string_s {
 };
 
 
+uint32_t njs_string_data_size(uint32_t size, uint32_t length);
+void njs_string_data_init(struct njs_string_s *string, uint32_t size,
+    uint32_t length);
+
+
 typedef struct {
     size_t    size;
     size_t    length;

diff --git a/src/test/njs_unit_test.c b/src/test/njs_unit_test.c
index f9b7266e6ead118b0d43571e762496c31380e09e..aac7e552b9920896250c7381320d9ecd3677a762 100644 (file)
--- a/src/test/njs_unit_test.c
+++ b/src/test/njs_unit_test.c
@@ -8296,6 +8296,18 @@ static njs_unit_test_t  njs_test[] =
     { njs_str("'r' !== '\\r'"),
       njs_str("true") },
 
+    { njs_str("'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaα'[32]"),
+      njs_str("α") },
+
+    { njs_str("'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaα'[33]"),
+      njs_str("α") },
+
+    { njs_str("'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaα'[34]"),
+      njs_str("α") },
+
+    { njs_str("'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaα'[35]"),
+      njs_str("α") },
+
     /* Octal escape sequences are not allowed in strict mode.*/
 
     { njs_str("'\\0a'"),