[gjs/201-null-out-arrays] arg: Handle case with null array and garbage length

[Philip Chimento <pchimento src gnome org>]

commit a9415fd8633f4b49939d4d79eff0db005507fdce
Author: Philip Chimento <philip chimento gmail com>
Date:   Mon Oct 8 23:29:58 2018 -0700

    arg: Handle case with null array and garbage length
    
    It happens sometimes in the case of an array out argument with a
    separate length argument, that C code passess a NULL array pointer with
    garbage as the length.
    
    In the particular case that caused the crash in the associated bug
    report, gtk_selection_data_get_targets() passed NULL as the array
    pointer and -1 as the length, which later got interpreted as an unsigned
    int and so caused a failure to allocate memory.
    
    I doubt that the C code should be doing this, but in any case we should
    not crash in this case. This adds a check for this situation to
    gjs_array_from_carray_internal() as well as to all the shortcuts and
    special cases called from there.
    
    Closes: #201

 gi/arg.cpp                | 19 +++++++++++++++++++
 gjs/byteArray.cpp         |  8 ++++++--
 gjs/jsapi-util-string.cpp |  6 ++++++
 3 files changed, 31 insertions(+), 2 deletions(-)
---
diff --git a/gi/arg.cpp b/gi/arg.cpp
index 0fd1b992..9e1b0d3d 100644
--- a/gi/arg.cpp
+++ b/gi/arg.cpp
@@ -965,6 +965,16 @@ gjs_array_from_flat_gvalue_array(JSContext             *context,
                                  JS::MutableHandleValue value)
 {
     GValue *values = (GValue *)array;
+
+    // a null array pointer takes precedence over whatever `length` says
+    if (!values) {
+        JSObject* jsarray = JS_NewArrayObject(context, 0);
+        if (!jsarray)
+            return false;
+        value.setObject(*jsarray);
+        return true;
+    }
+
     unsigned int i;
     JS::AutoValueVector elems(context);
     if (!elems.resize(length))
@@ -2288,6 +2298,15 @@ gjs_array_from_carray_internal (JSContext             *context,
     if (element_type == GI_TYPE_TAG_UNICHAR)
         return gjs_string_from_ucs4(context, (gunichar *) array, length, value_p);
 
+    // a null array pointer takes precedence over whatever `length` says
+    if (!array) {
+        JSObject* jsarray = JS_NewArrayObject(context, 0);
+        if (!jsarray)
+            return false;
+        value_p.setObject(*jsarray);
+        return true;
+    }
+
     JS::AutoValueVector elems(context);
     if (!elems.resize(length))
         g_error("Unable to resize vector");
diff --git a/gjs/byteArray.cpp b/gjs/byteArray.cpp
index 2dc6faf9..e2981c52 100644
--- a/gjs/byteArray.cpp
+++ b/gjs/byteArray.cpp
@@ -293,8 +293,12 @@ from_gbytes_func(JSContext *context,
 }
 
 JSObject* gjs_byte_array_from_data(JSContext* cx, size_t nbytes, void* data) {
-    JS::RootedObject array_buffer(
-        cx, JS_NewArrayBufferWithContents(cx, nbytes, g_memdup(data, nbytes)));
+    JS::RootedObject array_buffer(cx);
+    // a null data pointer takes precedence over whatever `nbytes` says
+    if (data)
+        array_buffer = JS_NewArrayBufferWithContents(cx, nbytes, g_memdup(data, nbytes));
+    else
+        array_buffer = JS_NewArrayBuffer(cx, 0);
     if (!array_buffer)
         return nullptr;
 
diff --git a/gjs/jsapi-util-string.cpp b/gjs/jsapi-util-string.cpp
index f3e2c02a..ef722522 100644
--- a/gjs/jsapi-util-string.cpp
+++ b/gjs/jsapi-util-string.cpp
@@ -288,6 +288,12 @@ gjs_string_from_ucs4(JSContext             *cx,
                      ssize_t                n_chars,
                      JS::MutableHandleValue value_p)
 {
+    // a null array pointer takes precedence over whatever `n_chars` says
+    if (!ucs4_string) {
+        value_p.setString(JS_GetEmptyString(cx));
+        return true;
+    }
+
     long u16_string_length;
     GError *error = NULL;