Sign Up
Log In
Log In
or
Sign Up
Places
All Projects
Status Monitor
Collapse sidebar
SUSE:SLE-15-SP1:Update
flatpak.28335
CVE-2023-28101.patch
Overview
Repositories
Revisions
Requests
Users
Attributes
Meta
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))
Locations
Projects
Search
Status Monitor
Help
OpenBuildService.org
Documentation
API Documentation
Code of Conduct
Contact
Support
@OBShq
Terms
openSUSE Build Service is sponsored by
The Open Build Service is an
openSUSE project
.
Sign Up
Log In
Places
Places
All Projects
Status Monitor
