CVE-2023-28101: Improper Encoding or Escaping of Output (CWE-116)
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.
CWE description: 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.
CVSS v3 base score: 4.3
Published: 2023-03-16T16:15Z
Repository: https://github.com/flatpak/flatpak
Language: C
Files changed: app/flatpak-builtins-info.c, app/flatpak-builtins-remote-info.c, app/flatpak-cli-transaction.c, common/flatpak-utils-private.h, common/flatpak-utils.c, tests/make-test-app.sh, tests/test-info.sh, tests/testcommon.c
Commit date: 2023-03-16 09:54:14 +0000
Commit message: Ensure special characters in permissions and metadata are escaped

This prevents someone from placing special characters in order to
manipulate the appearance of the permissions list.

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

Signed-off-by: Ryan Gonzalez <ryan.gonzalez@collabora.com>

Diff: diff --git a/app/flatpak-builtins-info.c b/app/flatpak-builtins-info.c
index 5f5445793d..ed9058369a 100644
--- a/app/flatpak-builtins-info.c
+++ b/app/flatpak-builtins-info.c
@@ -398,11 +398,13 @@ flatpak_builtin_info (int argc, char **argv, GCancellable *cancellable, GError *
           file = g_file_get_child (deploy_dir, "metadata");
 
           if (!g_file_load_contents (file, cancellable, &data, &data_size, NULL, error))
             return FALSE;
 
-          g_print ("%s", data);
+          flatpak_print_escaped_string (data,
+                                        FLATPAK_ESCAPE_ALLOW_NEWLINES
+                                        | FLATPAK_ESCAPE_DO_NOT_QUOTE);
         }
 
       if (opt_show_permissions || opt_file_access)
         {
           g_autoptr(FlatpakContext) app_context = NULL;
@@ -419,11 +421,13 @@ flatpak_builtin_info (int argc, char **argv, GCancellable *cancellable, GError *
               flatpak_context_save_metadata (app_context, TRUE, keyfile);
               contents = g_key_file_to_data (keyfile, NULL, error);
               if (contents == NULL)
                 return FALSE;
 
-              g_print ("%s", contents);
+              flatpak_print_escaped_string (contents,
+                                            FLATPAK_ESCAPE_ALLOW_NEWLINES
+                                            | FLATPAK_ESCAPE_DO_NOT_QUOTE);
             }
 
           if (opt_file_access)
             {
               g_autofree char *id = flatpak_decomposed_dup_id (ref);
diff --git a/app/flatpak-builtins-remote-info.c b/app/flatpak-builtins-remote-info.c
index c15eb8a2c5..bec93e89e8 100644
--- a/app/flatpak-builtins-remote-info.c
+++ b/app/flatpak-builtins-remote-info.c
@@ -429,11 +429,14 @@ flatpak_builtin_remote_info (int argc, char **argv, GCancellable *cancellable, G
           if (!first)
             g_print ("\n");
 
           if (opt_show_metadata)
             {
-              g_print ("%s", xa_metadata ? xa_metadata : "");
+              if (xa_metadata != NULL)
+                flatpak_print_escaped_string (xa_metadata,
+                                              FLATPAK_ESCAPE_ALLOW_NEWLINES
+                                              | FLATPAK_ESCAPE_DO_NOT_QUOTE);
               if (xa_metadata == NULL || !g_str_has_suffix (xa_metadata, "\n"))
                 g_print ("\n");
             }
 
           g_free (c);
diff --git a/app/flatpak-cli-transaction.c b/app/flatpak-cli-transaction.c
index 5a08d537bc..53593bfc20 100644
--- a/app/flatpak-cli-transaction.c
+++ b/app/flatpak-cli-transaction.c
@@ -1119,28 +1119,32 @@ static void
 print_perm_line (int        idx,
                  GPtrArray *items,
                  int        cols)
 {
   g_autoptr(GString) res = g_string_new (NULL);
+  g_autofree char *escaped_first_perm = NULL;
   int i;
 
-  g_string_append_printf (res, "    [%d] %s", idx, (char *) items->pdata[0]);
+  escaped_first_perm = flatpak_escape_string (items->pdata[0], FLATPAK_ESCAPE_DEFAULT);
+  g_string_append_printf (res, "    [%d] %s", idx, escaped_first_perm);
 
   for (i = 1; i < items->len; i++)
     {
+      g_autofree char *escaped = flatpak_escape_string (items->pdata[i],
+                                                        FLATPAK_ESCAPE_DEFAULT);
       char *p;
       int len;
 
       p = strrchr (res->str, '\n');
       if (!p)
         p = res->str;
 
       len = (res->str + strlen (res->str)) - p;
-      if (len + strlen ((char *) items->pdata[i]) + 2 >= cols)
-        g_string_append_printf (res, ",\n        %s", (char *) items->pdata[i]);
+      if (len + strlen (escaped) + 2 >= cols)
+        g_string_append_printf (res, ",\n        %s", escaped);
       else
-        g_string_append_printf (res, ", %s", (char *) items->pdata[i]);
+        g_string_append_printf (res, ", %s", escaped);
     }
 
   g_print ("%s\n", res->str);
 }
 
diff --git a/common/flatpak-utils-private.h b/common/flatpak-utils-private.h
index ceecb5c692..1ded8afce8 100644
--- a/common/flatpak-utils-private.h
+++ b/common/flatpak-utils-private.h
@@ -924,10 +924,21 @@ int flatpak_envp_cmp (const void *p1,
 gboolean flatpak_str_is_integer (const char *s);
 
 gboolean flatpak_uri_equal (const char *uri1,
                             const char *uri2);
 
+typedef enum {
+  FLATPAK_ESCAPE_DEFAULT        = 0,
+  FLATPAK_ESCAPE_ALLOW_NEWLINES = 1 << 0,
+  FLATPAK_ESCAPE_DO_NOT_QUOTE   = 1 << 1,
+} FlatpakEscapeFlags;
+
+char * flatpak_escape_string (const char        *s,
+                              FlatpakEscapeFlags flags);
+void   flatpak_print_escaped_string (const char        *s,
+                                     FlatpakEscapeFlags flags);
+
 gboolean running_under_sudo (void);
 
 #define FLATPAK_MESSAGE_ID "c7b39b1e006b464599465e105b361485"
 
 #endif /* __FLATPAK_UTILS_H__ */
diff --git a/common/flatpak-utils.c b/common/flatpak-utils.c
index 53b73ff0f9..fa8c465a79 100644
--- a/common/flatpak-utils.c
+++ b/common/flatpak-utils.c
@@ -611,11 +611,11 @@ load_kernel_module_list (void)
 {
   GHashTable *modules = g_hash_table_new_full (g_str_hash, g_str_equal, g_free, NULL);
   g_autofree char *modules_data = NULL;
   g_autoptr(GError) error = NULL;
   char *start, *end;
-  
+
   if (!g_file_get_contents ("/proc/modules", &modules_data, NULL, &error))
     {
       g_info ("Failed to read /proc/modules: %s", error->message);
       return modules;
     }
@@ -9220,10 +9220,90 @@ flatpak_uri_equal (const char *uri1,
     uri2_norm = g_strdup (uri2);
 
   return g_strcmp0 (uri1_norm, uri2_norm) == 0;
 }
 
+static gboolean
+is_char_safe (gunichar c)
+{
+  return g_unichar_isgraph (c) || c == ' ';
+}
+
+static gboolean
+should_hex_escape (gunichar           c,
+                   FlatpakEscapeFlags flags)
+{
+  if ((flags & FLATPAK_ESCAPE_ALLOW_NEWLINES) && c == '\n')
+    return FALSE;
+
+  return !is_char_safe (c);
+}
+
+static void
+append_hex_escaped_character (GString *result,
+                              gunichar c)
+{
+  if (c <= 0xFF)
+    g_string_append_printf (result, "\\x%02X", c);
+  else if (c <= 0xFFFF)
+    g_string_append_printf (result, "\\u%04X", c);
+  else
+    g_string_append_printf (result, "\\U%08X", c);
+}
+
+char *
+flatpak_escape_string (const char        *s,
+                       FlatpakEscapeFlags flags)
+{
+  g_autoptr(GString) res = g_string_new ("");
+  gboolean did_escape = FALSE;
+
+  while (*s)
+    {
+      gunichar c = g_utf8_get_char_validated (s, -1);
+      if (c == (gunichar)-2 || c == (gunichar)-1)
+        {
+          /* Need to convert to unsigned first, to avoid negative chars becoming
+             huge gunichars. */
+          append_hex_escaped_character (res, (unsigned char)*s++);
+          did_escape = TRUE;
+          continue;
+        }
+      else if (should_hex_escape (c, flags))
+        {
+          append_hex_escaped_character (res, c);
+          did_escape = TRUE;
+        }
+      else if (c == '\\' || (!(flags & FLATPAK_ESCAPE_DO_NOT_QUOTE) && c == '\''))
+        {
+          g_string_append_printf (res, "\\%c", (char) c);
+          did_escape = TRUE;
+        }
+      else
+        g_string_append_unichar (res, c);
+
+      s = g_utf8_find_next_char (s, NULL);
+    }
+
+  if (did_escape && !(flags & FLATPAK_ESCAPE_DO_NOT_QUOTE))
+    {
+      g_string_prepend_c (res, '\'');
+      g_string_append_c (res, '\'');
+    }
+
+  return g_string_free (g_steal_pointer (&res), FALSE);
+}
+
+void
+flatpak_print_escaped_string (const char        *s,
+                              FlatpakEscapeFlags flags)
+{
+  g_autofree char *escaped = flatpak_escape_string (s, flags);
+  g_print ("%s", escaped);
+}
+
+
 gboolean
 running_under_sudo (void)
 {
   const char *sudo_command_env = g_getenv ("SUDO_COMMAND");
   g_auto(GStrv) split_command = NULL;
diff --git a/tests/make-test-app.sh b/tests/make-test-app.sh
index afa11a6b21..06cae5f920 100755
--- a/tests/make-test-app.sh
+++ b/tests/make-test-app.sh
@@ -38,10 +38,18 @@ if [ x${REQUIRED_VERSION-} != x ]; then
 cat >> ${DIR}/metadata <<EOF
 required-flatpak=$REQUIRED_VERSION
 EOF
 fi
 
+if [ x${INCLUDE_SPECIAL_CHARACTER-} != x ]; then
+TAB=$'\t'
+cat >> ${DIR}/metadata <<EOF
+[Environment]
+A=x${TAB}y
+EOF
+fi
+
 cat >> ${DIR}/metadata <<EOF
 [Extension $APP_ID.Locale]
 directory=share/runtime/locale
 autodelete=true
 locale-subset=true
diff --git a/tests/test-info.sh b/tests/test-info.sh
index 4a247bdb48..2158c11b10 100644
--- a/tests/test-info.sh
+++ b/tests/test-info.sh
@@ -4,26 +4,34 @@ set -euo pipefail
 
 . $(dirname $0)/libtest.sh
 
 skip_revokefs_without_fuse
 
-echo "1..7"
+echo "1..8"
 
-setup_repo
+INCLUDE_SPECIAL_CHARACTER=1 setup_repo
 install_repo
 
 COMMIT=`${FLATPAK} ${U} info --show-commit org.test.Hello`
 
 ${FLATPAK} info -rcos  org.test.Hello > info
 
 assert_file_has_content info "^app/org\.test\.Hello/$(flatpak --default-arch)/master test-repo ${COMMIT}"
 
 ok "info -rcos"
 
+${FLATPAK} info --show-metadata  org.test.Hello > info
+
+# CVE-2023-28101
+assert_file_has_content info "name=org\.test\.Hello"
+assert_file_has_content info "^A=x\\\\x09y"
+
+ok "info --show-metadata"
+
 ${FLATPAK} info --show-permissions  org.test.Hello > info
 
-assert_file_empty info
+assert_file_has_content info "^A=x\\\\x09y"
 
 ok "info --show-permissions"
 
 ${FLATPAK} info --show-location  org.test.Hello > info
 
diff --git a/tests/testcommon.c b/tests/testcommon.c
index c527b20eb5..d8ad09c745 100644
--- a/tests/testcommon.c
+++ b/tests/testcommon.c
@@ -1835,10 +1835,48 @@ test_parse_x11_display (void)
           g_test_message ("-> successfully parsed");
         }
     }
 }
 
+typedef struct {
+  const char        *in;
+  FlatpakEscapeFlags flags;
+  const char        *out;
+} EscapeData;
+
+static EscapeData escapes[] = {
+  {"abc def", FLATPAK_ESCAPE_DEFAULT, "abc def"},
+  {"やあ", FLATPAK_ESCAPE_DEFAULT, "やあ"},
+  {"\033[;1m", FLATPAK_ESCAPE_DEFAULT, "'\\x1B[;1m'"},
+  // non-printable U+061C
+  {"\u061C", FLATPAK_ESCAPE_DEFAULT, "'\\u061C'"},
+  // non-printable U+1343F
+  {"\xF0\x93\x90\xBF", FLATPAK_ESCAPE_DEFAULT, "'\\U0001343F'"},
+  // invalid utf-8
+  {"\xD8\1", FLATPAK_ESCAPE_DEFAULT, "'\\xD8\\x01'"},
+  {"\b \n abc ' \\", FLATPAK_ESCAPE_DEFAULT, "'\\x08 \\x0A abc \\' \\\\'"},
+  {"\b \n abc ' \\", FLATPAK_ESCAPE_DO_NOT_QUOTE, "\\x08 \\x0A abc ' \\\\"},
+  {"abc\tdef\n\033[;1m ghi\b", FLATPAK_ESCAPE_ALLOW_NEWLINES | FLATPAK_ESCAPE_DO_NOT_QUOTE,
+   "abc\\x09def\n\\x1B[;1m ghi\\x08"},
+};
+
+/* CVE-2023-28101 */
+static void
+test_string_escape (void)
+{
+  gsize idx;
+
+  for (idx = 0; idx < G_N_ELEMENTS (escapes); idx++)
+    {
+      EscapeData *data = &escapes[idx];
+      g_autofree char *ret = NULL;
+
+      ret = flatpak_escape_string (data->in, data->flags);
+      g_assert_cmpstr (ret, ==, data->out);
+    }
+}
+
 int
 main (int argc, char *argv[])
 {
   int res;
 
@@ -1868,10 +1906,11 @@ main (int argc, char *argv[])
   g_test_add_func ("/common/envp-cmp", test_envp_cmp);
   g_test_add_func ("/common/needs-quoting", test_needs_quoting);
   g_test_add_func ("/common/quote-argv", test_quote_argv);
   g_test_add_func ("/common/str-is-integer", test_str_is_integer);
   g_test_add_func ("/common/parse-x11-display", test_parse_x11_display);
+  g_test_add_func ("/common/string-escape", test_string_escape);
 
   g_test_add_func ("/app/looks-like-branch", test_looks_like_branch);
   g_test_add_func ("/app/columns", test_columns);
   g_test_add_func ("/app/string-ellipsize", test_string_ellipsize);
   g_test_add_func ("/app/table", test_table);

Vulnerable code:           g_print ("%s", data);
              g_print ("%s", contents);
              g_print ("%s", xa_metadata ? xa_metadata : "");
  g_string_append_printf (res, "    [%d] %s", idx, (char *) items->pdata[0]);
      if (len + strlen ((char *) items->pdata[i]) + 2 >= cols)
        g_string_append_printf (res, ",\n        %s", (char *) items->pdata[i]);
        g_string_append_printf (res, ", %s", (char *) items->pdata[i]);
  
echo "1..7"
setup_repo
assert_file_empty info
Fixed code:           flatpak_print_escaped_string (data,
                                        FLATPAK_ESCAPE_ALLOW_NEWLINES
                                        | FLATPAK_ESCAPE_DO_NOT_QUOTE);
              flatpak_print_escaped_string (contents,
                                            FLATPAK_ESCAPE_ALLOW_NEWLINES
                                            | FLATPAK_ESCAPE_DO_NOT_QUOTE);
              if (xa_metadata != NULL)
                flatpak_print_escaped_string (xa_metadata,
                                              FLATPAK_ESCAPE_ALLOW_NEWLINES
                                              | FLATPAK_ESCAPE_DO_NOT_QUOTE);
  g_autofree char *escaped_first_perm = NULL;
  escaped_first_perm = flatpak_escape_string (items->pdata[0], FLATPAK_ESCAPE_DEFAULT);
  g_string_append_printf (res, "    [%d] %s", idx, escaped_first_perm);
      g_autofree char *escaped = flatpak_escape_string (items->pdata[i],
                                                        FLATPAK_ESCAPE_DEFAULT);
      if (len + strlen (escaped) + 2 >= cols)
        g_string_append_printf (res, ",\n        %s", escaped);
        g_string_append_printf (res, ", %s", escaped);
typedef enum {
  FLATPAK_ESCAPE_DEFAULT        = 0,
  FLATPAK_ESCAPE_ALLOW_NEWLINES = 1 << 0,
  FLATPAK_ESCAPE_DO_NOT_QUOTE   = 1 << 1,
} FlatpakEscapeFlags;

char * flatpak_escape_string (const char        *s,
                              FlatpakEscapeFlags flags);
void   flatpak_print_escaped_string (const char        *s,
                                     FlatpakEscapeFlags flags);


static gboolean
is_char_safe (gunichar c)
{
  return g_unichar_isgraph (c) || c == ' ';
}

static gboolean
should_hex_escape (gunichar           c,
                   FlatpakEscapeFlags flags)
{
  if ((flags & FLATPAK_ESCAPE_ALLOW_NEWLINES) && c == '\n')
    return FALSE;

  return !is_char_safe (c);
}

static void
append_hex_escaped_character (GString *result,
                              gunichar c)
{
  if (c <= 0xFF)
    g_string_append_printf (result, "\\x%02X", c);
  else if (c <= 0xFFFF)
    g_string_append_printf (result, "\\u%04X", c);
  else
    g_string_append_printf (result, "\\U%08X", c);
}

char *
flatpak_escape_string (const char        *s,
                       FlatpakEscapeFlags flags)
{
  g_autoptr(GString) res = g_string_new ("");
  gboolean did_escape = FALSE;

  while (*s)
    {
      gunichar c = g_utf8_get_char_validated (s, -1);
      if (c == (gunichar)-2 || c == (gunichar)-1)
        {
          /* Need to convert to unsigned first, to avoid negative chars becoming
             huge gunichars. */
          append_hex_escaped_character (res, (unsigned char)*s++);
          did_escape = TRUE;
          continue;
        }
      else if (should_hex_escape (c, flags))
        {
          append_hex_escaped_character (res, c);
          did_escape = TRUE;
        }
      else if (c == '\\' || (!(flags & FLATPAK_ESCAPE_DO_NOT_QUOTE) && c == '\''))
        {
          g_string_append_printf (res, "\\%c", (char) c);
          did_escape = TRUE;
        }
      else
        g_string_append_unichar (res, c);

      s = g_utf8_find_next_char (s, NULL);
    }

  if (did_escape && !(flags & FLATPAK_ESCAPE_DO_NOT_QUOTE))
    {
      g_string_prepend_c (res, '\'');
      g_string_append_c (res, '\'');
    }

  return g_string_free (g_steal_pointer (&res), FALSE);
}

void
flatpak_print_escaped_string (const char        *s,
                              FlatpakEscapeFlags flags)
{
  g_autofree char *escaped = flatpak_escape_string (s, flags);
  g_print ("%s", escaped);
}


if [ x${INCLUDE_SPECIAL_CHARACTER-} != x ]; then
TAB=$'\t'
cat >> ${DIR}/metadata <<EOF
[Environment]
A=x${TAB}y
EOF
fi

echo "1..8"
INCLUDE_SPECIAL_CHARACTER=1 setup_repo
${FLATPAK} info --show-metadata  org.test.Hello > info

# CVE-2023-28101
assert_file_has_content info "name=org\.test\.Hello"
assert_file_has_content info "^A=x\\\\x09y"

ok "info --show-metadata"

assert_file_has_content info "^A=x\\\\x09y"
typedef struct {
  const char        *in;
  FlatpakEscapeFlags flags;
  const char        *out;
} EscapeData;

static EscapeData escapes[] = {
  {"abc def", FLATPAK_ESCAPE_DEFAULT, "abc def"},
  {"やあ", FLATPAK_ESCAPE_DEFAULT, "やあ"},
  {"\033[;1m", FLATPAK_ESCAPE_DEFAULT, "'\\x1B[;1m'"},
  // non-printable U+061C
  {"\u061C", FLATPAK_ESCAPE_DEFAULT, "'\\u061C'"},
  // non-printable U+1343F
  {"\xF0\x93\x90\xBF", FLATPAK_ESCAPE_DEFAULT, "'\\U0001343F'"},
  // invalid utf-8
  {"\xD8\1", FLATPAK_ESCAPE_DEFAULT, "'\\xD8\\x01'"},
  {"\b \n abc ' \\", FLATPAK_ESCAPE_DEFAULT, "'\\x08 \\x0A abc \\' \\\\'"},
  {"\b \n abc ' \\", FLATPAK_ESCAPE_DO_NOT_QUOTE, "\\x08 \\x0A abc ' \\\\"},
  {"abc\tdef\n\033[;1m ghi\b", FLATPAK_ESCAPE_ALLOW_NEWLINES | FLATPAK_ESCAPE_DO_NOT_QUOTE,
   "abc\\x09def\n\\x1B[;1m ghi\\x08"},
};

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

  for (idx = 0; idx < G_N_ELEMENTS (escapes); idx++)
    {
      EscapeData *data = &escapes[idx];
      g_autofree char *ret = NULL;

      ret = flatpak_escape_string (data->in, data->flags);
      g_assert_cmpstr (ret, ==, data->out);
    }
}

  g_test_add_func ("/common/string-escape", test_string_escape);