File CVE-2023-28101.patch of Package flatpak.28335
From 6cac99dafe6003c8a4bd5666341c217876536869 Mon Sep 17 00:00:00 2001
From: Ryan Gonzalez <ryan.gonzalez@collabora.com>
Date: Sat, 4 Mar 2023 16:23:37 -0600
Subject: [PATCH 1/3] 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>
---
app/flatpak-builtins-info.c | 8 ++-
app/flatpak-builtins-remote-info.c | 5 +-
app/flatpak-cli-transaction.c | 12 +++--
common/flatpak-utils-private.h | 11 ++++
common/flatpak-utils.c | 82 +++++++++++++++++++++++++++++-
tests/make-test-app.sh | 8 +++
tests/test-info.sh | 14 +++--
tests/testcommon.c | 39 ++++++++++++++
8 files changed, 168 insertions(+), 11 deletions(-)
Index: flatpak-1.2.3/app/flatpak-builtins-info.c
===================================================================
--- flatpak-1.2.3.orig/app/flatpak-builtins-info.c
+++ flatpak-1.2.3/app/flatpak-builtins-info.c
@@ -415,7 +415,9 @@ flatpak_builtin_info (int argc, char **a
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)
@@ -436,7 +438,9 @@ flatpak_builtin_info (int argc, char **a
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)
Index: flatpak-1.2.3/app/flatpak-builtins-remote-info.c
===================================================================
--- flatpak-1.2.3.orig/app/flatpak-builtins-remote-info.c
+++ flatpak-1.2.3/app/flatpak-builtins-remote-info.c
@@ -404,7 +404,10 @@ flatpak_builtin_remote_info (int argc, c
g_print ("\n");
if (opt_show_metadata)
- g_print ("%s", xa_metadata);
+ if (xa_metadata != NULL)
+ flatpak_print_escaped_string (xa_metadata,
+ FLATPAK_ESCAPE_ALLOW_NEWLINES
+ | FLATPAK_ESCAPE_DO_NOT_QUOTE);
g_free (c);
c = g_steal_pointer (&p);
Index: flatpak-1.2.3/app/flatpak-cli-transaction.c
===================================================================
--- flatpak-1.2.3.orig/app/flatpak-cli-transaction.c
+++ flatpak-1.2.3/app/flatpak-cli-transaction.c
@@ -516,8 +516,13 @@ end_of_lifed (FlatpakTransaction *transa
msg = g_strdup_printf (_("Info: %s is end-of-life, in preference of %s"),
flatpak_ref_get_name (rref), rebase);
else if (reason)
- msg = g_strdup_printf (_("Info: %s is end-of-life, with reason: %s\n"),
- flatpak_ref_get_name (rref), reason);
+ {
+ g_autofree char *escaped_reason = flatpak_escape_string (reason,
+ FLATPAK_ESCAPE_ALLOW_NEWLINES |
+ FLATPAK_ESCAPE_DO_NOT_QUOTE);
+ msg = g_strdup_printf (_("Info: %s is end-of-life, with reason: %s\n"),
+ flatpak_ref_get_name (rref), escaped_reason);
+ }
if (flatpak_fancy_output ())
{
@@ -638,12 +643,16 @@ print_perm_line (int idx,
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;
@@ -652,10 +661,10 @@ print_perm_line (int idx,
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);
Index: flatpak-1.2.3/common/flatpak-utils-private.h
===================================================================
--- flatpak-1.2.3.orig/common/flatpak-utils-private.h
+++ flatpak-1.2.3/common/flatpak-utils-private.h
@@ -698,7 +698,7 @@ gboolean flatpak_allocate_tmpdir (int
gboolean flatpak_yes_no_prompt (gboolean default_yes,
const char *prompt,
...) G_GNUC_PRINTF (2, 3);
-
+
long flatpak_number_prompt (gboolean default_yes,
int min,
int max,
@@ -733,6 +733,20 @@ gboolean flatpak_check_required_version
int flatpak_levenshtein_distance (const char *s, const char *t);
-#define FLATPAK_MESSAGE_ID "c7b39b1e006b464599465e105b361485"
+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 flatpak_validate_path_characters (const char *path,
+ GError **error);
+
+#define FLATPAK_MESSAGE_ID "c7b39b1e006b464599465e105b361485"
#endif /* __FLATPAK_UTILS_H__ */
Index: flatpak-1.2.3/common/flatpak-utils.c
===================================================================
--- flatpak-1.2.3.orig/common/flatpak-utils.c
+++ flatpak-1.2.3/common/flatpak-utils.c
@@ -995,7 +995,7 @@ flatpak_is_valid_branch (const char *str
len = strlen (string);
if (G_UNLIKELY (len == 0))
{
- flatpak_fail_error (error, FLATPAK_ERROR_INVALID_NAME,
+ flatpak_fail_error (error, FLATPAK_ERROR_INVALID_NAME,
_("Branch can't be empty"));
goto out;
}
@@ -1005,7 +1005,7 @@ flatpak_is_valid_branch (const char *str
s = string;
if (G_UNLIKELY (!is_valid_initial_branch_character (*s)))
{
- flatpak_fail_error (error, FLATPAK_ERROR_INVALID_NAME,
+ flatpak_fail_error (error, FLATPAK_ERROR_INVALID_NAME,
_("Branch can't start with %c"), *s);
goto out;
}
@@ -1015,7 +1015,7 @@ flatpak_is_valid_branch (const char *str
{
if (G_UNLIKELY (!is_valid_branch_character (*s)))
{
- flatpak_fail_error (error, FLATPAK_ERROR_INVALID_NAME,
+ flatpak_fail_error (error, FLATPAK_ERROR_INVALID_NAME,
_("Branch can't contain %c"), *s);
goto out;
}
@@ -1630,7 +1630,7 @@ flatpak_find_deploy_for_ref_in (GPtrArra
for (i = 0; deploy == NULL && i < dirs->len; i++)
{
FlatpakDir *dir = g_ptr_array_index (dirs, i);
-
+
flatpak_log_dir_access (dir);
g_clear_error (&my_error);
deploy = flatpak_dir_load_deployed (dir, ref, commit, cancellable, &my_error);
@@ -6508,3 +6508,122 @@ out:
return datetime;
}
#endif
+
+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);
+}
+
+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);
+}
+
+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
+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;
+}
Index: flatpak-1.2.3/tests/make-test-app.sh
===================================================================
--- flatpak-1.2.3.orig/tests/make-test-app.sh
+++ flatpak-1.2.3/tests/make-test-app.sh
@@ -33,6 +33,14 @@ 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 org.test.Hello.Locale]
directory=share/runtime/locale
Index: flatpak-1.2.3/tests/test-info.sh
===================================================================
--- flatpak-1.2.3.orig/tests/test-info.sh
+++ flatpak-1.2.3/tests/test-info.sh
@@ -4,9 +4,9 @@ set -euo pipefail
. $(dirname $0)/libtest.sh
-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`
@@ -17,9 +17,17 @@ assert_file_has_content info "^app/org.t
echo "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"
echo "ok info --show-permissions"
Index: flatpak-1.2.3/tests/testcommon.c
===================================================================
--- flatpak-1.2.3.orig/tests/testcommon.c
+++ flatpak-1.2.3/tests/testcommon.c
@@ -343,18 +343,18 @@ test_parse_numbers (void)
numbers = flatpak_parse_numbers ("1", 0, 10);
assert_numbers (numbers, 1, 0);
g_clear_pointer (&numbers, g_free);
-
+
numbers = flatpak_parse_numbers ("1 3 2", 0, 10);
assert_numbers (numbers, 1, 3, 2, 0);
g_clear_pointer (&numbers, g_free);
-
+
numbers = flatpak_parse_numbers ("1-3", 0, 10);
assert_numbers (numbers, 1, 2, 3, 0);
g_clear_pointer (&numbers, g_free);
-
+
numbers = flatpak_parse_numbers ("1", 2, 4);
g_assert_null (numbers);
-
+
numbers = flatpak_parse_numbers ("2-6", 2, 4);
g_assert_null (numbers);
@@ -396,23 +396,23 @@ test_subpaths_merge (void)
res = flatpak_subpaths_merge (NULL, bla);
assert_strv_equal (res, bla);
g_clear_pointer (&res, g_strfreev);
-
+
res = flatpak_subpaths_merge (bla, NULL);
assert_strv_equal (res, bla);
g_clear_pointer (&res, g_strfreev);
-
+
res = flatpak_subpaths_merge (empty, bla);
assert_strv_equal (res, empty);
g_clear_pointer (&res, g_strfreev);
-
+
res = flatpak_subpaths_merge (bla, empty);
assert_strv_equal (res, empty);
g_clear_pointer (&res, g_strfreev);
-
+
res = flatpak_subpaths_merge (buba, bla);
assert_strv_equal (res, bubabla);
g_clear_pointer (&res, g_strfreev);
-
+
res = flatpak_subpaths_merge (bla, buba);
assert_strv_equal (res, bubabla);
g_clear_pointer (&res, g_strfreev);
@@ -485,7 +485,7 @@ test_parse_appdata (void)
gboolean res;
char *name;
char *comment;
-
+
res = flatpak_parse_appdata (appdata1, "org.test.Hello", &names, &comments, &version, &license);
g_assert_true (res);
g_assert_cmpstr (version, ==, "0.0.1");
@@ -950,6 +950,78 @@ test_parse_datetime (void)
g_assert_false (ret);
}
+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'"},
+ /* U+061C ARABIC LETTER MARK, non-printable */
+ {"\u061C", FLATPAK_ESCAPE_DEFAULT, "'\\u061C'"},
+ /* U+1343F EGYPTIAN HIEROGLYPH END WALLED ENCLOSURE, non-printable and
+ * outside BMP */
+ {"\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);
+ }
+}
+
+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);
+ }
+}
+
int
main (int argc, char *argv[])
{
@@ -973,6 +1045,8 @@ main (int argc, char *argv[])
g_test_add_func ("/common/lang-from-locale", test_lang_from_locale);
g_test_add_func ("/common/appdata", test_parse_appdata);
g_test_add_func ("/common/name-matching", test_name_matching);
+ g_test_add_func ("/common/string-escape", test_string_escape);
+ g_test_add_func ("/common/validate-path-characters", test_validate_path_characters);
g_test_add_func ("/app/looks-like-branch", test_looks_like_branch);
g_test_add_func ("/app/columns", test_columns);
Index: flatpak-1.2.3/common/flatpak-context.c
===================================================================
--- flatpak-1.2.3.orig/common/flatpak-context.c
+++ flatpak-1.2.3/common/flatpak-context.c
@@ -475,11 +475,16 @@ flatpak_context_apply_generic_policy (Fl
g_ptr_array_free (new, FALSE));
}
-static void
+static gboolean
flatpak_context_set_persistent (FlatpakContext *context,
- const char *path)
+ const char *path,
+ GError **error)
{
+ if (!flatpak_validate_path_characters (path, error))
+ return FALSE;
+
g_hash_table_insert (context->persistent, g_strdup (path), GINT_TO_POINTER (1));
+ return TRUE;
}
static gboolean
@@ -745,6 +750,9 @@ static gboolean
flatpak_context_verify_filesystem (const char *filesystem_and_mode,
GError **error)
{
+ if (!flatpak_validate_path_characters (filesystem_and_mode, error))
+ return FALSE;
+
g_autofree char *filesystem = parse_filesystem_flags (filesystem_and_mode, NULL);
if (strcmp (filesystem, "host") == 0)
@@ -1247,8 +1255,7 @@ option_persist_cb (const gchar *option_n
{
FlatpakContext *context = data;
- flatpak_context_set_persistent (context, value);
- return TRUE;
+ return flatpak_context_set_persistent (context, value, error);
}
static gboolean option_no_desktop_deprecated;
@@ -1436,9 +1443,22 @@ flatpak_context_load_metadata (FlatpakCo
for (i = 0; filesystems[i] != NULL; i++)
{
+ g_autoptr(GError) local_error = NULL;
const char *fs = parse_negated (filesystems[i], &remove);
- if (!flatpak_context_verify_filesystem (fs, NULL))
- g_debug ("Unknown filesystem type %s", filesystems[i]);
+ if (!flatpak_context_verify_filesystem (fs, &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);
+ }
+ }
else
{
if (remove)
@@ -1457,7 +1477,8 @@ flatpak_context_load_metadata (FlatpakCo
return FALSE;
for (i = 0; persistent[i] != NULL; i++)
- flatpak_context_set_persistent (context, persistent[i]);
+ if (!flatpak_context_set_persistent (context, persistent[i], error))
+ return FALSE;
}
if (g_key_file_has_group (metakey, FLATPAK_METADATA_GROUP_SESSION_BUS_POLICY))
