CVE-2021-43860: Incorrect Default Permissions (CWE-276)
value: Flatpak is a Linux application sandboxing and distribution framework. Prior to versions 1.12.3 and 1.10.6, Flatpak doesn't properly validate that the permissions displayed to the user for an app at install time match the actual permissions granted to the app at runtime, in the case that there's a null byte in the metadata file of an app. Therefore apps can grant themselves permissions without the consent of the user. Flatpak shows permissions to the user during install by reading them from the "xa.metadata" key in the commit metadata. This cannot contain a null terminator, because it is an untrusted GVariant. Flatpak compares these permissions to the *actual* metadata, from the "metadata" file to ensure it wasn't lied to. However, the actual metadata contents are loaded in several places where they are read as simple C-style strings. That means that, if the metadata file includes a null terminator, only the content of the file from *before* the terminator gets compared to xa.metadata. Thus, any permissions that appear in the metadata file after a null terminator are applied at runtime but not shown to the user. So maliciously crafted apps can give themselves hidden permissions. Users who have Flatpaks installed from untrusted sources are at risk in case the Flatpak has a maliciously crafted metadata file, either initially or in an update. This issue is patched in versions 1.12.3 and 1.10.6. As a workaround, users can manually check the permissions of installed apps by checking the metadata file or the xa.metadata key on the commit metadata.
CWE description: During installation, installed file permissions are set to allow anyone to modify those files.
CVSS v3 base score: 8.6
Published: 2022-01-12T22:15Z
Repository: https://github.com/flatpak/flatpak
Language: C
Files changed: common/flatpak-transaction.c
Commit date: 2022-01-12 19:48:16 +0100
Commit message: 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.)

Diff: diff --git a/common/flatpak-transaction.c b/common/flatpak-transaction.c
index a7e24d8f7..61fea4456 100644
--- a/common/flatpak-transaction.c
+++ b/common/flatpak-transaction.c
@@ -2968,16 +2968,17 @@ emit_eol_and_maybe_skip (FlatpakTransaction          *self,
   previous_ids[0] = id;
 
   g_signal_emit (self, signals[END_OF_LIFED_WITH_REBASE], 0, op->remote, flatpak_decomposed_get_ref (op->ref), op->eol, op->eol_rebase, previous_ids, &op->skip);
 }
 
-static void
+static gboolean
 mark_op_resolved (FlatpakTransactionOperation *op,
                   const char                  *commit,
                   GFile                       *sideload_path,
                   GBytes                      *metadata,
-                  GBytes                      *old_metadata)
+                  GBytes                      *old_metadata,
+                  GError                     **error)
 {
   g_debug ("marking op %s:%s resolved to %s", kind_to_str (op->kind), flatpak_decomposed_get_ref (op->ref), commit ? commit : "-");
 
   g_assert (op != NULL);
 
@@ -2995,83 +2996,97 @@ mark_op_resolved (FlatpakTransactionOperation *op,
     op->resolved_sideload_path = g_object_ref (sideload_path);
 
   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", flatpak_decomposed_get_ref (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,
+                                   "Metadata for %s is invalid", flatpak_decomposed_get_ref (op->ref));
+
+      op->resolved_metadata = g_bytes_ref (metadata);
+      op->resolved_metakey = g_steal_pointer (&metakey);
     }
   if (old_metadata)
     {
       g_autoptr(GKeyFile) metakey = g_key_file_new ();
       if (g_key_file_load_from_bytes (metakey, old_metadata, G_KEY_FILE_NONE, NULL))
         {
           op->resolved_old_metadata = g_bytes_ref (old_metadata);
           op->resolved_old_metakey = g_steal_pointer (&metakey);
         }
       else
-        g_message ("Warning: Failed to parse old metadata for %s\n", flatpak_decomposed_get_ref (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", flatpak_decomposed_get_ref (op->ref));
+        }
     }
+
+  return TRUE;
 }
 
-static void
+static gboolean
 resolve_op_end (FlatpakTransaction *self,
                 FlatpakTransactionOperation *op,
                 const char *checksum,
                 GFile *sideload_path,
-                GBytes *metadata_bytes)
+                GBytes *metadata_bytes,
+                GError **error)
 {
   g_autoptr(GBytes) old_metadata_bytes = NULL;
 
   old_metadata_bytes = load_deployed_metadata (self, op->ref, NULL, NULL);
-  mark_op_resolved (op, checksum, sideload_path, metadata_bytes, old_metadata_bytes);
+  if (!mark_op_resolved (op, checksum, sideload_path, metadata_bytes, old_metadata_bytes, error))
+    return FALSE;
   emit_eol_and_maybe_skip (self, op);
+  return TRUE;
  }
 
 
-static void
+static gboolean
 resolve_op_from_commit (FlatpakTransaction *self,
                         FlatpakTransactionOperation *op,
                         const char *checksum,
                         GFile *sideload_path,
-                        GVariant *commit_data)
+                        GVariant *commit_data,
+                        GError **error)
 {
   g_autoptr(GBytes) metadata_bytes = NULL;
   g_autoptr(GVariant) commit_metadata = NULL;
   const char *xa_metadata = NULL;
   guint64 download_size = 0;
   guint64 installed_size = 0;
 
   commit_metadata = g_variant_get_child_value (commit_data, 0);
   g_variant_lookup (commit_metadata, "xa.metadata", "&s", &xa_metadata);
   if (xa_metadata == NULL)
-    g_message ("Warning: No xa.metadata in local commit %s ref %s", checksum, flatpak_decomposed_get_ref (op->ref));
-  else
-    metadata_bytes = g_bytes_new (xa_metadata, strlen (xa_metadata));
+    return flatpak_fail_error (error, FLATPAK_ERROR_INVALID_DATA,
+                               "No xa.metadata in local commit %s ref %s",
+                               checksum, flatpak_decomposed_get_ref (op->ref));
+
+  metadata_bytes = g_bytes_new (xa_metadata, strlen (xa_metadata));
 
   if (g_variant_lookup (commit_metadata, "xa.download-size", "t", &download_size))
     op->download_size = GUINT64_FROM_BE (download_size);
   if (g_variant_lookup (commit_metadata, "xa.installed-size", "t", &installed_size))
     op->installed_size = GUINT64_FROM_BE (installed_size);
 
   g_variant_lookup (commit_metadata, OSTREE_COMMIT_META_KEY_ENDOFLIFE, "s", &op->eol);
   g_variant_lookup (commit_metadata, OSTREE_COMMIT_META_KEY_ENDOFLIFE_REBASE, "s", &op->eol_rebase);
 
-  resolve_op_end (self, op, checksum, sideload_path, metadata_bytes);
+  return resolve_op_end (self, op, checksum, sideload_path, metadata_bytes, error);
 }
 
+/* NOTE: In case of non-available summary this returns FALSE with a
+ * NULL error, but for other error cases it will be set.
+ */
 static gboolean
 try_resolve_op_from_metadata (FlatpakTransaction *self,
                               FlatpakTransactionOperation *op,
                               const char *checksum,
                               GFile *sideload_path,
-                              FlatpakRemoteState *state)
+                              FlatpakRemoteState *state,
+                              GError **error)
 {
   g_autoptr(GBytes) metadata_bytes = NULL;
   guint64 download_size = 0;
   guint64 installed_size = 0;
   const char *metadata = NULL;
@@ -3107,12 +3122,11 @@ try_resolve_op_from_metadata (FlatpakTransaction *self,
       op->eol = g_strdup (var_metadata_lookup_string (sparse_cache, FLATPAK_SPARSE_CACHE_KEY_ENDOFLINE, NULL));
       op->eol_rebase = g_strdup (var_metadata_lookup_string (sparse_cache, FLATPAK_SPARSE_CACHE_KEY_ENDOFLINE_REBASE, NULL));
       op->token_type = GINT32_FROM_LE (var_metadata_lookup_int32 (sparse_cache, FLATPAK_SPARSE_CACHE_KEY_TOKEN_TYPE, op->token_type));
     }
 
-  resolve_op_end (self, op, checksum, sideload_path, metadata_bytes);
-  return TRUE;
+  return resolve_op_end (self, op, checksum, sideload_path, metadata_bytes, error);
 }
 
 static gboolean
 op_may_need_token (FlatpakTransactionOperation *op)
 {
@@ -3151,11 +3165,12 @@ resolve_ops (FlatpakTransaction *self,
            * request_required_tokens() we were normalized away even though not fully resolved.
            * For example we got the checksum but need to auth to get the commit, but the
            * checksum we got was the version already installed.
            */
           g_assert (op->resolved_commit != NULL);
-          mark_op_resolved (op, op->resolved_commit, NULL, NULL, NULL);
+          if (!mark_op_resolved (op, op->resolved_commit, NULL, NULL, NULL, error))
+            return FALSE;
           continue;
         }
 
       if (op->kind == FLATPAK_TRANSACTION_OPERATION_UNINSTALL)
         {
@@ -3165,18 +3180,20 @@ resolve_ops (FlatpakTransaction *self,
           if (metadata_bytes == NULL)
             {
               op->skip = TRUE;
               continue;
             }
-          mark_op_resolved (op, checksum, NULL, metadata_bytes, NULL);
+          if (!mark_op_resolved (op, checksum, NULL, metadata_bytes, NULL, error))
+            return FALSE;
           continue;
         }
 
       if (op->kind == FLATPAK_TRANSACTION_OPERATION_INSTALL_BUNDLE)
         {
           g_assert (op->commit != NULL);
-          mark_op_resolved (op, op->commit, NULL, op->external_metadata, NULL);
+          if (!mark_op_resolved (op, op->commit, NULL, op->external_metadata, NULL, error))
+            return FALSE;
           continue;
         }
 
       /* op->kind is INSTALL or UPDATE */
 
@@ -3203,11 +3220,12 @@ resolve_ops (FlatpakTransaction *self,
           g_autoptr(GVariant) commit_data = flatpak_dir_read_latest_commit (priv->dir, op->remote, op->ref,
                                                                             &checksum, NULL, error);
           if (commit_data == NULL)
             return FALSE;
 
-          resolve_op_from_commit (self, op, checksum, NULL, commit_data);
+          if (!resolve_op_from_commit (self, op, checksum, NULL, commit_data, error))
+            return FALSE;
         }
       else
         {
           g_autoptr(GError) local_error = NULL;
           g_autoptr(GFile) sideload_path = NULL;
@@ -3262,13 +3280,20 @@ resolve_ops (FlatpakTransaction *self,
                   checksum = g_steal_pointer (&local_checksum);
                 }
             }
 
           /* First try to resolve via metadata (if remote is available and its metadata matches the commit version) */
-          if (!try_resolve_op_from_metadata (self, op, checksum, sideload_path, state))
+          if (!try_resolve_op_from_metadata (self, op, checksum, sideload_path, state, &local_error))
             {
-              /* Else try to load the commit object.
+              if (local_error)
+                {
+                  /* Actual error, not just missing from summary */
+                  g_propagate_error (error, g_steal_pointer (&local_error));
+                  return FALSE;
+                }
+
+              /* Missing from summary, try to load the commit object.
                * Note, we don't have a token here, so this will not work for authenticated apps.
                * We handle this by catching the 401 http status and retrying. */
               g_autoptr(GVariant) commit_data = NULL;
               VarRefInfoRef ref_info;
 
@@ -3300,11 +3325,12 @@ resolve_ops (FlatpakTransaction *self,
                     }
                   g_propagate_error (error, g_steal_pointer (&local_error));
                   return FALSE;
                 }
 
-              resolve_op_from_commit (self, op, checksum, sideload_path, commit_data);
+              if (!resolve_op_from_commit (self, op, checksum, sideload_path, commit_data, error))
+                return FALSE;
             }
         }
     }
 
   return TRUE;

Vulnerable code: static void
                  GBytes                      *old_metadata)
      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", flatpak_decomposed_get_ref (op->ref));
        g_message ("Warning: Failed to parse old metadata for %s\n", flatpak_decomposed_get_ref (op->ref));
static void
                GBytes *metadata_bytes)
  mark_op_resolved (op, checksum, sideload_path, metadata_bytes, old_metadata_bytes);
static void
                        GVariant *commit_data)
    g_message ("Warning: No xa.metadata in local commit %s ref %s", checksum, flatpak_decomposed_get_ref (op->ref));
  else
    metadata_bytes = g_bytes_new (xa_metadata, strlen (xa_metadata));
  resolve_op_end (self, op, checksum, sideload_path, metadata_bytes);
                              FlatpakRemoteState *state)
  resolve_op_end (self, op, checksum, sideload_path, metadata_bytes);
  return TRUE;
          mark_op_resolved (op, op->resolved_commit, NULL, NULL, NULL);
          mark_op_resolved (op, checksum, NULL, metadata_bytes, NULL);
          mark_op_resolved (op, op->commit, NULL, op->external_metadata, NULL);
          resolve_op_from_commit (self, op, checksum, NULL, commit_data);
          if (!try_resolve_op_from_metadata (self, op, checksum, sideload_path, state))
              /* Else try to load the commit object.
              resolve_op_from_commit (self, op, checksum, sideload_path, commit_data);
Fixed code: static gboolean
                  GBytes                      *old_metadata,
                  GError                     **error)
      if (!g_key_file_load_from_bytes (metakey, metadata, G_KEY_FILE_NONE, NULL))
        return flatpak_fail_error (error, FLATPAK_ERROR_INVALID_DATA,
                                   "Metadata for %s is invalid", flatpak_decomposed_get_ref (op->ref));

      op->resolved_metadata = g_bytes_ref (metadata);
      op->resolved_metakey = g_steal_pointer (&metakey);
        {
          /* 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", flatpak_decomposed_get_ref (op->ref));
        }

  return TRUE;
static gboolean
                GBytes *metadata_bytes,
                GError **error)
  if (!mark_op_resolved (op, checksum, sideload_path, metadata_bytes, old_metadata_bytes, error))
    return FALSE;
  return TRUE;
static gboolean
                        GVariant *commit_data,
                        GError **error)
    return flatpak_fail_error (error, FLATPAK_ERROR_INVALID_DATA,
                               "No xa.metadata in local commit %s ref %s",
                               checksum, flatpak_decomposed_get_ref (op->ref));

  metadata_bytes = g_bytes_new (xa_metadata, strlen (xa_metadata));
  return resolve_op_end (self, op, checksum, sideload_path, metadata_bytes, error);
/* NOTE: In case of non-available summary this returns FALSE with a
 * NULL error, but for other error cases it will be set.
 */
                              FlatpakRemoteState *state,
                              GError **error)
  return resolve_op_end (self, op, checksum, sideload_path, metadata_bytes, error);
          if (!mark_op_resolved (op, op->resolved_commit, NULL, NULL, NULL, error))
            return FALSE;
          if (!mark_op_resolved (op, checksum, NULL, metadata_bytes, NULL, error))
            return FALSE;
          if (!mark_op_resolved (op, op->commit, NULL, op->external_metadata, NULL, error))
            return FALSE;
          if (!resolve_op_from_commit (self, op, checksum, NULL, commit_data, error))
            return FALSE;
          if (!try_resolve_op_from_metadata (self, op, checksum, sideload_path, state, &local_error))
              if (local_error)
                {
                  /* Actual error, not just missing from summary */
                  g_propagate_error (error, g_steal_pointer (&local_error));
                  return FALSE;
                }

              /* Missing from summary, try to load the commit object.
              if (!resolve_op_from_commit (self, op, checksum, sideload_path, commit_data, error))
                return FALSE;