Fix the conversion of a property key to an array index

Previously, a double was converted to uint32_t before its range was tested,
so a property key that is NaN, negative or larger than 2^31 made the
conversion undefined.  UndefinedBehaviorSanitizer reported it for every VM
creation, where an environment variable name is atomized.

The fix is to test the range first and to share one njs_number_is_atom_index()
helper between the four places that repeated the pattern.

diff --git a/src/njs_atom.c b/src/njs_atom.c
index b987246c9d2d01234e44460eca1a2454a533d34c..2d94318eeec655a337190f0e92d3cd5a7bcf54f1 100644 (file)
--- a/src/njs_atom.c
+++ b/src/njs_atom.c
@@ -235,9 +235,8 @@ njs_atom_atomize_key(njs_vm_t *vm, njs_value_t *value)
     switch (value->type) {
     case NJS_STRING:
         num = njs_key_to_index(value);
-        u32 = (uint32_t) num;
 
-        if (njs_fast_path(u32 == num && (u32 < 0x80000000)
+        if (njs_fast_path(njs_number_is_atom_index(num, &u32)
                           && !(num == 0 && signbit(num))))
         {
             value->atom_id = njs_number_atom(u32);
@@ -262,9 +261,8 @@ njs_atom_atomize_key(njs_vm_t *vm, njs_value_t *value)
 
     case NJS_NUMBER:
         num = njs_number(value);
-        u32 = (uint32_t) num;
 
-        if (njs_fast_path(u32 == num && (u32 < 0x80000000))) {
+        if (njs_fast_path(njs_number_is_atom_index(num, &u32))) {
             value->atom_id = njs_number_atom(u32);
 
         } else {

diff --git a/src/njs_number.h b/src/njs_number.h
index 2659105ef2c67abd30dd16524db67dc8fd48aa60..3a03302a1b23e4a4252498f75f1da245c23f07ad 100644 (file)
--- a/src/njs_number.h
+++ b/src/njs_number.h
@@ -65,6 +65,24 @@ njs_key_is_integer_index(double num, const njs_value_t *value)
 }
 
 
+/*
+ * The range is tested before the conversion: converting a NaN or an out of
+ * range double to an unsigned integer is undefined.
+ */
+
+njs_inline njs_bool_t
+njs_number_is_atom_index(double num, uint32_t *index)
+{
+    if (njs_slow_path(!(num >= 0 && num < 0x80000000))) {
+        return 0;
+    }
+
+    *index = (uint32_t) num;
+
+    return (double) *index == num;
+}
+
+
 njs_inline int64_t
 njs_number_to_integer(double num)
 {

diff --git a/src/njs_vmcode.c b/src/njs_vmcode.c
index 3d453f3e2f648d230d36cbf9c08562e207d6a7c6..d09450b19483fbd3617241c7bc64f36e0c0d0b86 100644 (file)
--- a/src/njs_vmcode.c
+++ b/src/njs_vmcode.c
@@ -294,9 +294,8 @@ NEXT_LBL;
 
         if (value2->atom_id == NJS_ATOM_STRING_unknown) {
             num = njs_number(value2);
-            u32 = (uint32_t) num;
 
-            if (njs_fast_path(u32 == num && (u32 < 0x80000000))) {
+            if (njs_fast_path(njs_number_is_atom_index(num, &u32))) {
                 value2->atom_id = njs_number_atom(u32);
 
             } else {
@@ -1368,9 +1367,8 @@ NEXT_LBL;
 
         if (value2->atom_id == NJS_ATOM_STRING_unknown) {
             num = njs_number(value2);
-            u32 = (uint32_t) num;
 
-            if (njs_fast_path(u32 == num && (u32 < 0x80000000))) {
+            if (njs_fast_path(njs_number_is_atom_index(num, &u32))) {
                 value2->atom_id = njs_number_atom(u32);
 
             } else {