Suspected unterminated/malformed C-string/`gchar *` in `nemo_main_application_open()`?

[robertkirkman]

Distribution

Android (Termux)

Package version

6.4.5

Frequency

Only occasionally

Bug description

Hello, isn't this passing of a pointer to a standalone const char directly into g_strsplit() at this line an unterminated C-string, or at least a subtly incorrect datatype?

nemo/src/nemo-main-application.c

Lines 536 to 543 in 032ce13

	const char splitter = '=';
	g_debug ("Open called on the GApplication instance; %d files", n_files);
	if (!open_in_existing_window) {
	/* Check if local command line passed --geometry or --tabs */
	if (strlen (options) > 0) {
	gchar** split_options = g_strsplit (options, &splitter, 2);

GLib documentation states that g_strsplit()'s second argument takes a proper gchar *, not a pointer to an individual char.

https://docs.gtk.org/glib/func.strsplit.html

backtrace:

Thread 1 "nemo" received signal SIGSEGV, Segmentation fault.
0x0000007fb2495a70 in strlen () from /apex/com.android.runtime/lib64/bionic/libc.so
(gdb) bt
#0  0x0000007fb2495a70 in strlen () from /apex/com.android.runtime/lib64/bionic/libc.so
#1  0x0000007fb24e5d00 in vsscanf () from /apex/com.android.runtime/lib64/bionic/libc.so
#2  0x0000007fb24f7f84 in sscanf () from /apex/com.android.runtime/lib64/bionic/libc.so
#3  0x000000555566e550 in nemo_main_application_open (app=0x7fb1654130, files=0x7fb17e4d90, n_files=1, 
    options=<optimized out>) at ../src/src/nemo-main-application.c:547
#4  0x0000007fb54e5c04 in ?? () from /data/data/com.termux/files/usr/lib/libgio-2.0.so.0.8600.1
#5  0x0000007fb5a48228 in ?? () from /data/data/com.termux/files/usr/lib/libgobject-2.0.so.0.8600.1
#6  0x0000007fb5a47c2c in ?? () from /data/data/com.termux/files/usr/lib/libgobject-2.0.so.0.8600.1
#7  0x0000007fb5a5b6b4 in ?? () from /data/data/com.termux/files/usr/lib/libgobject-2.0.so.0.8600.1
#8  0x0000007fb5a5af78 in g_signal_emit_valist () from /data/data/com.termux/files/usr/lib/libgobject-2.0.so.0.8600.1
#9  0x0000007fb5a5b980 in g_signal_emit () from /data/data/com.termux/files/usr/lib/libgobject-2.0.so.0.8600.1
#10 0x0000007fb5536bd0 in g_application_open () from /data/data/com.termux/files/usr/lib/libgio-2.0.so.0.8600.1
#11 0x000000555566eda8 in nemo_main_application_local_command_line (application=<optimized out>, 
    arguments=<optimized out>, exit_status=<optimized out>) at ../src/src/nemo-main-application.c:843
#12 0x0000007fb5536d68 in g_application_run () from /data/data/com.termux/files/usr/lib/libgio-2.0.so.0.8600.1
#13 0x000000555566f8f0 in main (argc=1, argv=0x7fffffeac8) at ../src/src/nemo-main.c:102
(gdb) 

Steps to reproduce

1. Android 10 device suspected required to reproduce (not Android 11 or newer, or Android 9 or older), possibly specifically the proprietary One UI 2.5 OFW ROM of Samsung Galaxy S9 and Samsung Galaxy S9+
2. Install Termux and Termux:X11
3. Upgrade all packages using pkg upgrade
4. Install XFCE and Nemo using the command pkg install x11-repo followed by pkg install xfce nemo
5. Launch XFCE using LIBGL_ALWAYS_SOFTWARE=1 termux-x11 -xstartup xfce4-session &
6. Launch Nemo using DISPLAY=:0 nemo

Expected behavior

Nemo not crashing on Samsung Galaxy S9 and Samsung Galaxy S9+

Additional information

I've written this patch which appears to fix the problem when applied to Nemo running on my device:

Fixes https://github.com/termux/termux-packages/issues/26982
g_strsplit() takes arguments as gchar*, not char*
https://docs.gtk.org/glib/func.strsplit.html

--- a/src/nemo-main-application.c
+++ b/src/nemo-main-application.c
@@ -533,14 +533,14 @@ nemo_main_application_open (GApplication *app,
 	gboolean open_in_tabs = FALSE;
 	gchar *geometry = NULL;
 	gboolean open_in_existing_window = strcmp (options, "EXISTING_WINDOW") == 0;
-	const char splitter = '=';
+	gchar *splitter = g_strdup ("=");
 
 	g_debug ("Open called on the GApplication instance; %d files", n_files);
 
 	if (!open_in_existing_window) {
 		/* Check if local command line passed --geometry or --tabs */
 		if (strlen (options) > 0) {
-			gchar** split_options = g_strsplit (options, &splitter, 2);
+			gchar** split_options = g_strsplit (options, splitter, 2);
 			if (strcmp (split_options[0], "NULL") != 0) {
 				geometry = g_strdup (split_options[0]);
 			}
@@ -548,6 +548,7 @@ nemo_main_application_open (GApplication *app,
 			g_strfreev (split_options);
 		}
 	}
+	g_free (splitter);
 
 	DEBUG ("Open called on the GApplication instance; %d files, open in tabs: %s, geometry: '%s',"
            "open in existing window: %s",

There are also other ways to implement solutions, like for example maybe just passing a string literal "=" directly to the second argument of g_strsplit() instead of &splitter, or another way that was created by licy183:

--- a/src/nemo-main-application.c
+++ b/src/nemo-main-application.c
@@ -533,14 +533,14 @@ nemo_main_application_open (GApplication *app,
 	gboolean open_in_tabs = FALSE;
 	gchar *geometry = NULL;
 	gboolean open_in_existing_window = strcmp (options, "EXISTING_WINDOW") == 0;
-	const char splitter = '=';
+	gchar splitter[2] = {'=', '\0'};
 
 	g_debug ("Open called on the GApplication instance; %d files", n_files);
 
 	if (!open_in_existing_window) {
 		/* Check if local command line passed --geometry or --tabs */
 		if (strlen (options) > 0) {
-			gchar** split_options = g_strsplit (options, &splitter, 2);
+			gchar** split_options = g_strsplit (options, splitter, 2);
 			if (strcmp (split_options[0], "NULL") != 0) {
 				geometry = g_strdup (split_options[0]);
 			}