mate-session doesn't push 1024+ character environment variables into systemd (and dbus)

[twhitehead]

If mate-session encounters an environment variable of length 1024 or greater

$ FOO=0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef mate-session

it won't update it in the systemd user environment

$ systemctl --user show-environment | grep ^FOO

The following code

#include <glib.h>

int main() {
  char           **entry_names;
  int              i = 0;
  GVariantBuilder  builder;
  GRegex          *name_regex, *value_regex;
  GError          *error = 0;

  name_regex = g_regex_new ("^[a-zA-Z_][a-zA-Z0-9_]*$", G_REGEX_OPTIMIZE, 0, &error);

  if (name_regex == NULL) {
    return 1;
  }

  value_regex = g_regex_new ("^([[:blank:]]|[^[:cntrl:]])*$", G_REGEX_OPTIMIZE, 0, &error);

  if (value_regex == NULL) {
    return 1;
  }

  g_variant_builder_init (&builder, G_VARIANT_TYPE ("a{ss}"));
  for (entry_names = g_listenv (); entry_names[i] != NULL; i++) {
    const char *entry_name = entry_names[i];
    const char *entry_value = g_getenv (entry_name);

    if (!g_utf8_validate (entry_name, -1, NULL)) {
      g_print("g_utf8_validate entry_name failed: %s\n", entry_name);
      continue;
    }

    if (!g_regex_match (name_regex, entry_name, 0, NULL)) {
      g_print("g_regex_match entry_name failed: %s\n", entry_name);
      continue;
    }

    if (!g_utf8_validate (entry_value, -1, NULL)) {
      g_print("g_utf8_validate entry_value failed: %s\n", entry_name);
      continue;
    }

    if (!g_regex_match (value_regex, entry_value, 0, NULL)) {
      g_print("g_regex_match entry_value failed: %s\n", entry_name);
      continue;
    }

    g_variant_builder_add (&builder, "{ss}", entry_name, entry_value);
  }
  g_regex_unref (name_regex);
  g_regex_unref (value_regex);

  g_strfreev (entry_names);

  return 0;
}

is the first of the export routines. Compiling

$ gcc $(pkg-config --cflags --libs glib-2.0) test.c -o test

and running it with a 1024 or greater environment variable reveals the issue is that g_regex_match doesn't match when the string is 1024+

$ FOO=0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef ./test
g_regex_match entry_value failed: FOO

but it has no issues with 1023 characters

$ FOO=0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcde ./test

In my case the actual variable is PATH which is 1024+ due to some unfortunately long prefixes. I played a bit with the regex, and the length of what can be matched seems to depend on the regex.

[twhitehead]

Using (?: ... ) was actually one the alternatives that I tried. It fails too, just at different points.

Specifically, updating the test program to use ^([ \t\n]|[^[:cntrl:]])$ shows it fails at +1365 characters

FOO=0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdefFOO=0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0 ./test
g_regex_match entry_value failed: FOO

[twhitehead]

I thing this would do what you want

// No escape chararters but tab (0x09) and newline (0x0a) (accepts all multi-byte UTF8 too as they are all > 0x7f)
if ( strcspn(entry_value, "\x01\x02\x03\x04\x05\x06\x07\x08\x0b\x0c\x0d\x0e\x0f"
                          "\x10\x11\x12\x13\x14\x15\x16\x17\x18\x19\x1a\x1b\x1c\1x1d\x1e\x1f"
                          "\x7f") < strlen(entry_value) )
  continue;

[cwendling]

What about something like that? Just ditch the regexes altogether and parse that manually, it's pretty straightforward.

Suggested patch ⚠️ untested

diff --git a/mate-session/gsm-util.c b/mate-session/gsm-util.c
index a760fc3..b6bb408 100644
--- a/mate-session/gsm-util.c
+++ b/mate-session/gsm-util.c
@@ -501,6 +501,57 @@ gsm_util_update_activation_environment (const char  *variable,
         return environment_updated;
 }
 
+/* skips the name of an envvar ("[a-zA-Z_][a-zA-Z0-9_]*") */
+static const gchar *
+skip_env_name (const gchar *name)
+{
+        if (! name) {
+                return name;
+        }
+        if (! (g_ascii_isalpha(*name) || *name == '_')) {
+                return name;
+        }
+        do {
+                name++;
+        } while (g_ascii_isalnum(*name) || *name == '_');
+        return name;
+}
+
+/* checks `name` is an entirely valid envvar name */
+static gboolean
+validate_env_name (const gchar *name)
+{
+        const gchar *end = skip_env_name(name);
+        return end > name && *name == 0;
+}
+
+/* checks `value` is a valid envvar value ("^([[:blank:]]|[^[:cntrl]])*$",
+ * valid UTF-8) */
+static gboolean
+validate_env_value (const gchar *value)
+{
+        if (! g_utf8_validate(value, -1, NULL))
+                return FALSE;
+
+        for (; *value; value++) {
+                if (! g_ascii_isspace(*value) && g_ascii_iscntrl(*value)) {
+                        return FALSE;
+                }
+        }
+
+        return TRUE;
+}
+
+/* checks `name_and_value` is of the form <envvar-name>=<envvar-value> */
+static gboolean
+validate_env_name_value (const gchar *name_and_value)
+{
+        const gchar *name_end = skip_env_name(name_and_value);
+
+        return (name_end > name_and_value && *name_end == '=' &&
+                validate_env_value(name_end + 1));
+}
+
 gboolean
 gsm_util_export_activation_environment (GError     **error)
 {
@@ -509,7 +560,6 @@ gsm_util_export_activation_environment (GError     **error)
         char           **entry_names;
         int              i = 0;
         GVariantBuilder  builder;
-        GRegex          *name_regex, *value_regex;
         GVariant        *reply;
         GError          *bus_error = NULL;
 
@@ -519,39 +569,19 @@ gsm_util_export_activation_environment (GError     **error)
                 return FALSE;
         }
 
-        name_regex = g_regex_new ("^[a-zA-Z_][a-zA-Z0-9_]*$", G_REGEX_OPTIMIZE, 0, error);
-
-        if (name_regex == NULL) {
-                return FALSE;
-        }
-
-        value_regex = g_regex_new ("^([[:blank:]]|[^[:cntrl:]])*$", G_REGEX_OPTIMIZE, 0, error);
-
-        if (value_regex == NULL) {
-                return FALSE;
-        }
-
         g_variant_builder_init (&builder, G_VARIANT_TYPE ("a{ss}"));
         for (entry_names = g_listenv (); entry_names[i] != NULL; i++) {
                 const char *entry_name = entry_names[i];
                 const char *entry_value = g_getenv (entry_name);
 
-                if (!g_utf8_validate (entry_name, -1, NULL))
+                if (!validate_env_name(entry_name))
                     continue;
 
-                if (!g_regex_match (name_regex, entry_name, 0, NULL))
-                    continue;
-
-                if (!g_utf8_validate (entry_value, -1, NULL))
-                    continue;
-
-                if (!g_regex_match (value_regex, entry_value, 0, NULL))
+                if (!validate_env_value(entry_value))
                     continue;
 
                 g_variant_builder_add (&builder, "{ss}", entry_name, entry_value);
         }
-        g_regex_unref (name_regex);
-        g_regex_unref (value_regex);
 
         g_strfreev (entry_names);
 
@@ -587,7 +617,6 @@ gsm_util_export_user_environment (GError     **error)
         char           **entries;
         int              i = 0;
         GVariantBuilder  builder;
-        GRegex          *regex;
         GVariant        *reply;
         GError          *bus_error = NULL;
 
@@ -597,25 +626,15 @@ gsm_util_export_user_environment (GError     **error)
                 return FALSE;
         }
 
-        regex = g_regex_new ("^[a-zA-Z_][a-zA-Z0-9_]*=([[:blank:]]|[^[:cntrl:]])*$", G_REGEX_OPTIMIZE, 0, error);
-
-        if (regex == NULL) {
-                return FALSE;
-        }
-
         g_variant_builder_init (&builder, G_VARIANT_TYPE ("as"));
         for (entries = g_get_environ (); entries[i] != NULL; i++) {
                 const char *entry = entries[i];
 
-                if (!g_utf8_validate (entry, -1, NULL))
-                    continue;
-
-                if (!g_regex_match (regex, entry, 0, NULL))
+                if (! validate_env_name_value (entry))
                     continue;
 
                 g_variant_builder_add (&builder, "s", entry);
         }
-        g_regex_unref (regex);
 
         g_strfreev (entries);
 

This does not include the systemd conditionals from #323 but that would be easy to incorporate (just everything for the value except the UTF-8 validation).

[twhitehead]

Presumably you want to be incrementing value somewhere in there 😁

        while (*value) {
                if (! g_ascii_isspace(*value) && g_ascii_iscntrl(*value)) {
                        return FALSE;
                }
        }

[cwendling]

Presumably you want to be incrementing value somewhere in there 😁

🤦 fixed