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

3 files changed, 31 insertions, 2 deletions

diff --git a/gi/arg.cpp b/gi/arg.cpp
index 6b101341..af8a6bfe 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))
@@ -2283,6 +2293,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 5dacfe48..d06fb271 100644
--- a/gjs/byteArray.cpp
+++ b/gjs/byteArray.cpp
@@ -291,8 +291,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 03e8c4fd..f7ef4251 100644
--- a/gjs/jsapi-util-string.cpp
+++ b/gjs/jsapi-util-string.cpp
@@ -278,6 +278,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;