From d9a8f9d8ccc0b7c1135d0ecde006a75d25f66aee Mon Sep 17 00:00:00 2001 From: Alexander Larsson Date: Mon, 10 Jan 2022 16:43:08 +0100 Subject: [PATCH] Transaction: Fail the resolve if xa.metadata invalid or missing If we fail to parse xa.metadata from the summary cache or the commit xa.metadata we fail the resolve. If xa.metadata is missing in the commit we fail the resolve (it is always set in the summary cache, because summary update converts missing xa.metadata to "", so we either get that, or cache miss which leads to resolving from the commit. This means that op->resolved_metadata is always set during install and updates, which means we will show the app permissions. The transaction will also always make sure that this data actually matches what gets deployed. Before this change an invalid metadata in the summary cache could lead to a NULL resolved_metadata, which means we wouldn't print the app permissions, yet we would still deploy some metadata file that could have permissions. (NOTE: It would fail to deploy unless the xa.metadata in the commit matched the metadata file, but in this corner case we would't compare the summary and commit metadata, so they may differ.) Conflict:NA Reference:https://github.com/flatpak/flatpak/commit/d9a8f9d8ccc0b7c1135d0ecde006a75d25f66aee --- common/flatpak-transaction.c | 36 +++++++++++++++++++++--------------- 1 file changed, 21 insertions(+), 15 deletions(-) diff --git a/common/flatpak-transaction.c b/common/flatpak-transaction.c index 4e19e5d..2134a3e 100644 --- a/common/flatpak-transaction.c +++ b/common/flatpak-transaction.c @@ -1842,11 +1842,12 @@ load_deployed_metadata (FlatpakTransaction *self, const char *ref) return g_bytes_new_take (g_steal_pointer (&metadata_contents), metadata_contents_length); } -static void +static gboolean mark_op_resolved (FlatpakTransactionOperation *op, const char *commit, GBytes *metadata, - GBytes *old_metadata) + GBytes *old_metadata, + GError **error) { g_debug ("marking op %s:%s resolved to %s", kind_to_str (op->kind), op->ref, commit ? commit : "-"); @@ -1860,13 +1861,12 @@ mark_op_resolved (FlatpakTransactionOperation *op, if (metadata) { g_autoptr(GKeyFile) metakey = g_key_file_new (); - if (g_key_file_load_from_bytes (metakey, metadata, G_KEY_FILE_NONE, NULL)) - { - op->resolved_metadata = g_bytes_ref (metadata); - op->resolved_metakey = g_steal_pointer (&metakey); - } - else - g_message ("Warning: Failed to parse metadata for %s\n", op->ref); + if (!g_key_file_load_from_bytes (metakey, metadata, G_KEY_FILE_NONE, NULL)) + return flatpak_fail_error (error, FLATPAK_ERROR_INVALID_DATA, + _("Invalid .flatpakref: %s"), op->ref); + + op->resolved_metadata = g_bytes_ref (metadata); + op->resolved_metakey = g_steal_pointer (&metakey); } if (old_metadata) { @@ -1877,8 +1877,13 @@ mark_op_resolved (FlatpakTransactionOperation *op, op->resolved_old_metakey = g_steal_pointer (&metakey); } else - g_message ("Warning: Failed to parse old metadata for %s\n", op->ref); + { + /* This shouldn't happen, but a NULL old metadata is safe (all permisssions are considered new) */ + g_message ("Warning: Failed to parse old metadata for %s\n", op->ref); + } } + + return TRUE; } static gboolean @@ -1922,7 +1927,7 @@ resolve_p2p_ops (FlatpakTransaction *self, g_autoptr(GBytes) old_metadata_bytes = NULL; old_metadata_bytes = load_deployed_metadata (self, op->ref); - mark_op_resolved (op, resolve->resolved_commit, resolve->resolved_metadata, old_metadata_bytes); + mark_op_resolved (op, resolve->resolved_commit, resolve->resolved_metadata, old_metadata_bytes,error); } return TRUE; @@ -1960,14 +1965,15 @@ resolve_ops (FlatpakTransaction *self, /* We resolve to the deployed metadata, becasue we need it to uninstall related ops */ metadata_bytes = load_deployed_metadata (self, op->ref); - mark_op_resolved (op, NULL, metadata_bytes, NULL); + mark_op_resolved (op, NULL, metadata_bytes, NULL,error); continue; } if (op->kind == FLATPAK_TRANSACTION_OPERATION_INSTALL_BUNDLE) { g_assert (op->commit != NULL); - mark_op_resolved (op, op->commit, op->external_metadata, NULL); + if (!mark_op_resolved (op, op->commit, NULL, NULL, error)) + return FALSE; continue; } @@ -1993,7 +1999,7 @@ resolve_ops (FlatpakTransaction *self, metadata_bytes = g_bytes_new (xa_metadata, strlen (xa_metadata) + 1); old_metadata_bytes = load_deployed_metadata (self, op->ref); - mark_op_resolved (op, checksum, metadata_bytes, old_metadata_bytes); + mark_op_resolved (op, checksum, metadata_bytes, old_metadata_bytes,error); } else if (state->collection_id == NULL) /* In the non-p2p case we have all the info available in the summary, so use it */ { @@ -2031,7 +2037,7 @@ resolve_ops (FlatpakTransaction *self, metadata_bytes = g_bytes_new (metadata, strlen (metadata) + 1); old_metadata_bytes = load_deployed_metadata (self, op->ref); - mark_op_resolved (op, checksum, metadata_bytes, old_metadata_bytes); + mark_op_resolved (op, checksum, metadata_bytes, old_metadata_bytes,error); } else { -- 2.27.0