CVE-2023-28101 (C, published 2023-03-16T16:15Z)
Description: value: Flatpak is a system for building, distributing, and running sandboxed desktop applications on Linux. In versions prior to 1.10.8, 1.12.8, 1.14.4, and 1.15.4, if an attacker publishes a Flatpak app with elevated permissions, they can hide those permissions from users of the `flatpak(1)` command-line interface by setting other permissions to crafted values that contain non-printable control characters such as `ESC`. A fix is available in versions 1.10.8, 1.12.8, 1.14.4, and 1.15.4. As a workaround, use a GUI like GNOME Software rather than the command-line interface, or only install apps whose maintainers you trust.
Severity: nan (CVSS v3 base score: 4.3)
Weakness: CWE-116 \u2014 Improper Encoding or Escaping of Output
The product prepares a structured message for communication with another component, but encoding or escaping of the data is either missing or done incorrectly. As a result, the intended structure of the message is not preserved.
Repository: https://github.com/flatpak/flatpak
Files changed:
common/flatpak-context.c
common/flatpak-utils-private.h
common/flatpak-utils.c
tests/test-context.c
tests/testcommon.c
Commit message: Reject paths given to --filesystem/--persist with special characters

There isn't much in the way of legit reasons for this, but it's a
potential security footgun when displaying the text.

CVE-2023-28101, GHSA-h43h-fwqx-mpp8

Signed-off-by: Ryan Gonzalez <ryan.gonzalez@collabora.com>
Co-authored-by: Simon McVittie <smcv@collabora.com>


Vulnerable code:
static void
                                const char     *path)
  flatpak_context_set_persistent (context, value);
  return TRUE;
                                                 &filesystem, &mode, NULL))
            g_info ("Unknown filesystem type %s", filesystems[i]);
        flatpak_context_set_persistent (context, persistent[i]);
  g_autoptr(GError) local_error = NULL;
  va_start (ap, context);
  g_option_context_parse_strv (oc, &argv, &local_error);
  g_assert_no_error (local_error);
  // non-printable U+061C
  // non-printable U+1343F
  // invalid utf-8

Fixed code:

static gboolean
                                const char     *path,
                                GError        **error)
  if (!flatpak_validate_path_characters (path, error))
    return FALSE;

  return TRUE;
  if (!flatpak_validate_path_characters (filesystem_and_mode, error))
    return FALSE;

  return flatpak_context_set_persistent (context, value, error);
          g_autoptr(GError) local_error = NULL;
                                                 &filesystem, &mode, &local_error))
            {
              if (g_error_matches (local_error, G_IO_ERROR, G_IO_ERROR_INVALID_DATA))
                {
                  /* Invalid characters, so just hard-fail. */
                  g_propagate_error (error, g_steal_pointer (&local_error));
                  return FALSE;
                }
              else
                {
                  g_info ("Unknown filesystem type %s", filesystems[i]);
                  g_clear_error (&local_error);
                }
            }
        if (!flatpak_context_set_persistent (context, persistent[i], error))
          return FALSE;
gboolean flatpak_validate_path_characters (const char *path,
                                           GError    **error);

static char *
escape_character (gunichar c)
{
  g_autoptr(GString) res = g_string_new ("");
  append_hex_escaped_character (res, c);
  return g_string_free (g_steal_pointer (&res), FALSE);
}

gboolean
flatpak_validate_path_characters (const char *path,
                                  GError    **error)
{
  while (*path)
    {
      gunichar c = g_utf8_get_char_validated (path, -1);
      if (c == (gunichar)-1 || c == (gunichar)-2)
        {
          /* Need to convert to unsigned first, to avoid negative chars becoming
             huge gunichars. */
          g_autofree char *escaped_char = escape_character ((unsigned char)*path);
          g_autofree char *escaped = flatpak_escape_string (path, FLATPAK_ESCAPE_DEFAULT);
          g_set_error (error, G_IO_ERROR, G_IO_ERROR_INVALID_DATA,
                       "Non-UTF8 byte %s in path %s", escaped_char, escaped);
          return FALSE;
        }
      else if (!is_char_safe (c))
        {
          g_autofree char *escaped_char = escape_character (c);
          g_autofree char *escaped = flatpak_escape_string (path, FLATPAK_ESCAPE_DEFAULT);
          g_set_error (error, G_IO_ERROR, G_IO_ERROR_INVALID_DATA,
                       "Non-graphical character %s in path %s", escaped_char, escaped);
          return FALSE;
        }

      path = g_utf8_find_next_char (path, NULL);
    }

  return TRUE;
}
                                GError        **error,
                    GError        **error,
  va_start (ap, error);
  g_option_context_parse_strv (oc, &argv, error);
      g_autoptr(GError) local_error = NULL;
                          &local_error,
      g_assert_no_error (local_error);
                          &local_error,
      g_assert_no_error (local_error);
                          &local_error,
      g_assert_no_error (local_error);
                          &local_error,
      g_assert_no_error (local_error);
                          &local_error,
      g_assert_no_error (local_error);
                          &local_error,
      g_assert_no_error (local_error);
                          &local_error,
      g_assert_no_error (local_error);
const char *invalid_path_args[] = {
  "--filesystem=/\033[J:ro",
  "--filesystem=/\033[J",
  "--persist=\033[J",
};

/* CVE-2023-28101 */
static void
test_validate_path_args (void)
{
  gsize idx;

  for (idx = 0; idx < G_N_ELEMENTS (invalid_path_args); idx++)
    {
      g_autoptr(FlatpakContext) context = flatpak_context_new ();
      g_autoptr(GError) local_error = NULL;
      const char *path = invalid_path_args[idx];

      context_parse_args (context, &local_error, path, NULL);
      g_assert_error (local_error, G_IO_ERROR, G_IO_ERROR_INVALID_DATA);
      g_assert (strstr (local_error->message, "Non-graphical character"));
    }
}

typedef struct {
  const char *key;
  const char *value;
} PathValidityData;

PathValidityData invalid_path_meta[] = {
  {FLATPAK_METADATA_KEY_FILESYSTEMS, "\033[J"},
  {FLATPAK_METADATA_KEY_PERSISTENT, "\033[J"},
};

/* CVE-2023-28101 */
static void
test_validate_path_meta (void)
{
  gsize idx;

  for (idx = 0; idx < G_N_ELEMENTS (invalid_path_meta); idx++)
    {
      g_autoptr(FlatpakContext) context = flatpak_context_new ();
      g_autoptr(GKeyFile) metakey = g_key_file_new ();
      g_autoptr(GError) local_error = NULL;
      PathValidityData *data = &invalid_path_meta[idx];
      gboolean ret = FALSE;

      g_key_file_set_string_list (metakey, FLATPAK_METADATA_GROUP_CONTEXT,
                                  data->key, &data->value, 1);

      ret = flatpak_context_load_metadata (context, metakey, &local_error);
      g_assert_false (ret);
      g_assert_error (local_error, G_IO_ERROR, G_IO_ERROR_INVALID_DATA);
      g_assert (strstr (local_error->message, "Non-graphical character"));
    }

}

  g_test_add_func ("/context/validate-path-args", test_validate_path_args);
  g_test_add_func ("/context/validate-path-meta", test_validate_path_meta);
  /* U+061C ARABIC LETTER MARK, non-printable */
  /* U+1343F EGYPTIAN HIEROGLYPH END WALLED ENCLOSURE, non-printable and
   * outside BMP */
  /* invalid utf-8 */
typedef struct {
  const char *path;
  gboolean ret;
} PathValidityData;

static PathValidityData paths[] = {
  {"/a/b/../c.def", TRUE},
  {"やあ", TRUE},
  /* U+061C ARABIC LETTER MARK, non-printable */
  {"\u061C", FALSE},
  /* U+1343F EGYPTIAN HIEROGLYPH END WALLED ENCLOSURE, non-printable and
   * outside BMP */
  {"\xF0\x93\x90\xBF", FALSE},
  /* invalid utf-8 */
  {"\xD8\1", FALSE},
};

/* CVE-2023-28101 */
static void
test_validate_path_characters (void)
{
  gsize idx;

  for (idx = 0; idx < G_N_ELEMENTS (paths); idx++)
    {
      PathValidityData *data = &paths[idx];
      gboolean ret = FALSE;

      ret = flatpak_validate_path_characters (data->path, NULL);
      g_assert_cmpint (ret, ==, data->ret);
    }
}

  g_test_add_func ("/common/validate-path-characters", test_validate_path_characters);