From ba818f504c926baaf6e362be8159cfacf994310e Mon Sep 17 00:00:00 2001 From: Ryan Gonzalez Date: Thu, 23 Dec 2021 18:30:17 -0600 Subject: [PATCH] Fix metadata file contents after null terminators being ignored In particular, if a null terminator is placed inside the metadata file, Flatpak will only compare the text *before* it to the value of xa.metadata, but the full file will be parsed when permissions are set at runtime. This means that any app can include a null terminator in its permissions metadata, and Flatpak will only show the user the permissions *preceding* the terminator during install, but the permissions *after* the terminator are applied at runtime. Fixes GHSA-qpjc-vq3c-572j / CVE-2021-43860 Signed-off-by: Ryan Gonzalez Conflict:NA Reference:https://github.com/flatpak/flatpak/commit/ba818f504c926baaf6e362be8159cfacf994310e --- common/flatpak-dir.c | 5 +++-- common/flatpak-transaction.c | 4 ++-- common/flatpak-utils.c | 9 +++++---- 3 files changed, 10 insertions(+), 8 deletions(-) diff --git a/common/flatpak-dir.c b/common/flatpak-dir.c index 56bca24..5af4b4b 100644 --- a/common/flatpak-dir.c +++ b/common/flatpak-dir.c @@ -6608,6 +6608,7 @@ flatpak_dir_deploy (FlatpakDir *self, g_autoptr(GFile) metadata_file = NULL; g_autofree char *metadata_contents = NULL; g_autofree char *application_runtime = NULL; + gsize metadata_size = 0; gboolean is_app; if (!flatpak_dir_ensure_repo (self, cancellable, error)) @@ -6850,12 +6851,12 @@ flatpak_dir_deploy (FlatpakDir *self, metadata_file = g_file_resolve_relative_path (checkoutdir, "metadata"); if (g_file_load_contents (metadata_file, NULL, - &metadata_contents, NULL, NULL, NULL)) + &metadata_contents, &metadata_size, NULL, NULL)) { g_autoptr(GKeyFile) keyfile = g_key_file_new (); if (!g_key_file_load_from_data (keyfile, metadata_contents, - -1, + metadata_size, 0, error)) return FALSE; diff --git a/common/flatpak-transaction.c b/common/flatpak-transaction.c index 396d75c..4e19e5d 100644 --- a/common/flatpak-transaction.c +++ b/common/flatpak-transaction.c @@ -1604,7 +1604,7 @@ flatpak_transaction_add_ref (FlatpakTransaction *self, op = flatpak_transaction_add_op (self, remote, ref, subpaths, commit, bundle, kind); if (external_metadata) - op->external_metadata = g_bytes_new (external_metadata, strlen (external_metadata) + 1); + op->external_metadata = g_bytes_new (external_metadata, strlen (external_metadata)); return TRUE; } @@ -1839,7 +1839,7 @@ load_deployed_metadata (FlatpakTransaction *self, const char *ref) return NULL; } - return g_bytes_new_take (g_steal_pointer (&metadata_contents), metadata_contents_length + 1); + return g_bytes_new_take (g_steal_pointer (&metadata_contents), metadata_contents_length); } static void diff --git a/common/flatpak-utils.c b/common/flatpak-utils.c index 23b72d6..accf230 100644 --- a/common/flatpak-utils.c +++ b/common/flatpak-utils.c @@ -4674,6 +4674,7 @@ flatpak_pull_from_bundle (OstreeRepo *repo, GCancellable *cancellable, GError **error) { + gsize metadata_size = 0; g_autofree char *metadata_contents = NULL; g_autofree char *to_checksum = NULL; @@ -4691,6 +4692,8 @@ flatpak_pull_from_bundle (OstreeRepo *repo, if (metadata == NULL) return FALSE; + metadata_size = strlen (metadata_contents); + if (!ostree_repo_get_remote_option (repo, remote, "collection-id", NULL, &remote_collection_id, NULL)) remote_collection_id = NULL; @@ -4760,12 +4763,10 @@ flatpak_pull_from_bundle (OstreeRepo *repo, cancellable, error) < 0) return FALSE; - /* Null terminate */ - g_output_stream_write (G_OUTPUT_STREAM (data_stream), "\0", 1, NULL, NULL); - metadata_valid = metadata_contents != NULL && - strcmp (metadata_contents, g_memory_output_stream_get_data (data_stream)) == 0; + metadata_size == g_memory_output_stream_get_data_size (data_stream) && + memcmp (metadata_contents, g_memory_output_stream_get_data (data_stream), metadata_size) == 0; } else { -- 2.27.0