[njs] Added uint32_t overflow check for njs_array_alloc() function.

Alexander Borisov alexander.borisov at nginx.com
Fri Apr 19 17:12:03 UTC 2019


details:   https://hg.nginx.org/njs/rev/434c654ef638
branches:  
changeset: 912:434c654ef638
user:      Alexander Borisov <alexander.borisov at nginx.com>
date:      Fri Apr 19 17:48:39 2019 +0300
description:
Added uint32_t overflow check for njs_array_alloc() function.

diffstat:

 njs/njs_array.c          |  24 +++++++++++++++++-------
 njs/njs_array.h          |  10 +++++-----
 njs/njs_object.c         |   4 ++--
 njs/test/njs_unit_test.c |   6 ++++++
 4 files changed, 30 insertions(+), 14 deletions(-)

diffs (110 lines):

diff -r a6c82ddff460 -r 434c654ef638 njs/njs_array.c
--- a/njs/njs_array.c	Fri Apr 19 20:03:49 2019 +0800
+++ b/njs/njs_array.c	Fri Apr 19 17:48:39 2019 +0300
@@ -125,22 +125,26 @@ static njs_ret_t njs_array_prototype_sor
 
 
 nxt_noinline njs_array_t *
-njs_array_alloc(njs_vm_t *vm, uint32_t length, uint32_t spare)
+njs_array_alloc(njs_vm_t *vm, uint64_t length, uint32_t spare)
 {
     uint64_t     size;
     njs_array_t  *array;
 
+    if (nxt_slow_path(length > UINT32_MAX)) {
+        goto overflow;
+    }
+
+    size = length + spare;
+
+    if (nxt_slow_path(size > NJS_ARRAY_MAX_LENGTH)) {
+        goto memory_error;
+    }
+
     array = nxt_mp_alloc(vm->mem_pool, sizeof(njs_array_t));
     if (nxt_slow_path(array == NULL)) {
         goto memory_error;
     }
 
-    size = (uint64_t) length + spare;
-
-    if (nxt_slow_path((size * sizeof(njs_value_t)) >= UINT32_MAX)) {
-        goto memory_error;
-    }
-
     array->data = nxt_mp_align(vm->mem_pool, sizeof(njs_value_t),
                                size * sizeof(njs_value_t));
     if (nxt_slow_path(array->data == NULL)) {
@@ -164,6 +168,12 @@ memory_error:
     njs_memory_error(vm);
 
     return NULL;
+
+overflow:
+
+    njs_range_error(vm, "Invalid array length");
+
+    return NULL;
 }
 
 
diff -r a6c82ddff460 -r 434c654ef638 njs/njs_array.h
--- a/njs/njs_array.h	Fri Apr 19 20:03:49 2019 +0800
+++ b/njs/njs_array.h	Fri Apr 19 17:48:39 2019 +0300
@@ -8,14 +8,14 @@
 #define _NJS_ARRAY_H_INCLUDED_
 
 
-#define NJS_ARRAY_MAX_LENGTH     0xffffffff
-/* The maximum valid array index is the maximum array length minus 1. */
-#define NJS_ARRAY_INVALID_INDEX  NJS_ARRAY_MAX_LENGTH
+#define NJS_ARRAY_MAX_INDEX      0xffffffff
+#define NJS_ARRAY_INVALID_INDEX  NJS_ARRAY_MAX_INDEX
 
-#define NJS_ARRAY_SPARE  8
+#define NJS_ARRAY_SPARE          8
+#define NJS_ARRAY_MAX_LENGTH     (UINT32_MAX/ sizeof(njs_value_t))
 
 
-njs_array_t *njs_array_alloc(njs_vm_t *vm, uint32_t length, uint32_t spare);
+njs_array_t *njs_array_alloc(njs_vm_t *vm, uint64_t length, uint32_t spare);
 njs_ret_t njs_array_add(njs_vm_t *vm, njs_array_t *array, njs_value_t *value);
 njs_ret_t njs_array_string_add(njs_vm_t *vm, njs_array_t *array,
     const u_char *start, size_t size, size_t length);
diff -r a6c82ddff460 -r 434c654ef638 njs/njs_object.c
--- a/njs/njs_object.c	Fri Apr 19 20:03:49 2019 +0800
+++ b/njs/njs_object.c	Fri Apr 19 17:48:39 2019 +0300
@@ -338,7 +338,7 @@ njs_property_query(njs_vm_t *vm, njs_pro
         if (nxt_fast_path(!njs_is_null_or_undefined_or_boolean(property))) {
             index = njs_value_to_index(property);
 
-            if (nxt_fast_path(index < NJS_ARRAY_MAX_LENGTH)) {
+            if (nxt_fast_path(index < NJS_ARRAY_MAX_INDEX)) {
                 return njs_array_property_query(vm, pq, object->data.u.array,
                                                 index);
             }
@@ -459,7 +459,7 @@ njs_object_property_query(njs_vm_t *vm, 
                 switch (proto->type) {
                 case NJS_ARRAY:
                     index = njs_value_to_index(property);
-                    if (nxt_fast_path(index < NJS_ARRAY_MAX_LENGTH)) {
+                    if (nxt_fast_path(index < NJS_ARRAY_MAX_INDEX)) {
                         array = (njs_array_t *) proto;
                         return njs_array_property_query(vm, pq, array, index);
                     }
diff -r a6c82ddff460 -r 434c654ef638 njs/test/njs_unit_test.c
--- a/njs/test/njs_unit_test.c	Fri Apr 19 20:03:49 2019 +0800
+++ b/njs/test/njs_unit_test.c	Fri Apr 19 17:48:39 2019 +0300
@@ -7950,6 +7950,12 @@ static njs_unit_test_t  njs_test[] =
     { nxt_string("var a = Array(1111111111)"),
       nxt_string("MemoryError") },
 
+    { nxt_string("var x = Array(2**32)"),
+      nxt_string("RangeError: Invalid array length") },
+
+    { nxt_string("var x = Array(2**28)"),
+      nxt_string("MemoryError") },
+
     { nxt_string("var a = new Array(3); a"),
       nxt_string(",,") },
 


More information about the nginx-devel mailing list