File CVE-2021-43860.patch of Package flatpak.25785
diff -urpN flatpak-1.2.3.orig/common/flatpak-dir.c flatpak-1.2.3/common/flatpak-dir.c
--- flatpak-1.2.3.orig/common/flatpak-dir.c 2022-08-31 15:19:04.784529834 -0500
+++ flatpak-1.2.3/common/flatpak-dir.c 2022-09-01 10:08:57.818529522 -0500
@@ -1003,19 +1003,28 @@ static gboolean
validate_commit_metadata (GVariant *commit_data,
const char *ref,
const char *required_metadata,
- gboolean require_xa_metadata,
+ gsize required_metadata_size,
GError **error)
{
g_autoptr(GVariant) commit_metadata = NULL;
+ g_autoptr(GVariant) xa_metadata_v = NULL;
const char *xa_metadata = NULL;
+ gsize xa_metadata_size = 0;
commit_metadata = g_variant_get_child_value (commit_data, 0);
if (commit_metadata != NULL)
- g_variant_lookup (commit_metadata, "xa.metadata", "&s", &xa_metadata);
+ {
+ xa_metadata_v = g_variant_lookup_value (commit_metadata,
+ "xa.metadata",
+ G_VARIANT_TYPE_STRING);
+ if (xa_metadata_v)
+ xa_metadata = g_variant_get_string (xa_metadata_v, &xa_metadata_size);
+ }
- if ((xa_metadata == NULL && require_xa_metadata) ||
- (xa_metadata != NULL && g_strcmp0 (required_metadata, xa_metadata) != 0))
+ if (xa_metadata == NULL ||
+ xa_metadata_size != required_metadata_size ||
+ memcmp (xa_metadata, required_metadata, xa_metadata_size) != 0)
{
g_set_error (error, G_IO_ERROR, G_IO_ERROR_PERMISSION_DENIED,
_("Commit metadata for %s not matching expected metadata"), ref);
@@ -4806,8 +4815,12 @@ flatpak_dir_pull (FlatpakDir
{
g_autoptr(GVariant) commit_data = NULL;
if (!ostree_repo_load_commit (repo, rev, &commit_data, NULL, error) ||
- !validate_commit_metadata (commit_data, ref, (const char *)g_bytes_get_data (require_metadata, NULL), TRUE, error))
- return FALSE;
+ !validate_commit_metadata (commit_data,
+ ref,
+ (const char *)g_bytes_get_data (require_metadata, NULL),
+ g_bytes_get_size (require_metadata),
+ error))
+ goto out;
}
if (!flatpak_dir_pull_extra_data (self, repo,
@@ -6949,9 +6962,9 @@ flatpak_dir_deploy (FlatpakDir
g_auto(GLnxLockFile) lock = { 0, };
g_autoptr(GFile) metadata_file = NULL;
g_autofree char *metadata_contents = NULL;
+ gsize metadata_size = 0;
g_auto(GStrv) ref_parts = NULL;
gboolean is_app;
- gboolean is_oci;
if (!flatpak_dir_ensure_repo (self, cancellable, error))
return FALSE;
@@ -7194,11 +7207,12 @@ flatpak_dir_deploy (FlatpakDir
keyfile = g_key_file_new ();
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))
{
if (!g_key_file_load_from_data (keyfile,
metadata_contents,
- -1,
+ metadata_size,
0, error))
return FALSE;
@@ -7209,11 +7223,8 @@ flatpak_dir_deploy (FlatpakDir
/* Check the metadata in the commit to make sure it matches the actual
* deployed metadata, in case we relied on the one in the commit for
* a decision
- * Note: For historical reason we don't enforce commits to contain xa.metadata
- * since this was lacking in fedora builds.
*/
- is_oci = flatpak_dir_get_remote_oci (self, origin);
- if (!validate_commit_metadata (commit_data, ref, metadata_contents, !is_oci, error))
+ if (!validate_commit_metadata (commit_data, ref, metadata_contents, metadata_size, error))
return FALSE;
dotref = g_file_resolve_relative_path (checkoutdir, "files/.ref");
@@ -7941,6 +7952,13 @@ flatpak_dir_ensure_bundle_remote (Flatpa
if (metadata == NULL)
return NULL;
+ /* If we rely on metadata (to e.g. print permissions), check it exists before creating the remote */
+ if (out_metadata && fp_metadata == NULL)
+ {
+ flatpak_fail_error (error, FLATPAK_ERROR_INVALID_DATA, "No metadata in bundler header");
+ return NULL;
+ }
+
gpg_data = extra_gpg_data ? extra_gpg_data : included_gpg_data;
parts = flatpak_decompose_ref (ref, error);
diff -urpN flatpak-1.2.3.orig/common/flatpak-transaction.c flatpak-1.2.3/common/flatpak-transaction.c
--- flatpak-1.2.3.orig/common/flatpak-transaction.c 2022-08-31 15:19:04.784529834 -0500
+++ flatpak-1.2.3/common/flatpak-transaction.c 2022-09-01 10:10:36.199543664 -0500
@@ -1746,7 +1746,7 @@ flatpak_transaction_add_ref (FlatpakTran
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;
}
@@ -1988,14 +1988,15 @@ load_deployed_metadata (FlatpakTransacti
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
+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 : "-");
@@ -2009,13 +2010,12 @@ mark_op_resolved (FlatpakTransactionOper
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,
+ "Metadata for %s is invalid", op->ref);
+
+ op->resolved_metadata = g_bytes_ref (metadata);
+ op->resolved_metakey = g_steal_pointer (&metakey);
}
if (old_metadata)
{
@@ -2026,8 +2026,13 @@ mark_op_resolved (FlatpakTransactionOper
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
@@ -2074,7 +2079,8 @@ resolve_p2p_ops (FlatpakTransaction *sel
op->installed_size = resolve->installed_size;
old_metadata_bytes = load_deployed_metadata (self, op->ref);
- mark_op_resolved (op, resolve->resolved_commit, resolve->resolved_metadata, old_metadata_bytes);
+ if (!mark_op_resolved (op, resolve->resolved_commit, resolve->resolved_metadata, old_metadata_bytes, error))
+ return FALSE;
}
return TRUE;
@@ -2114,14 +2120,16 @@ 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);
+ if (!mark_op_resolved (op, 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, op->external_metadata, NULL);
+ if (!mark_op_resolved (op, op->commit, op->external_metadata, NULL, error))
+ return FALSE;
continue;
}
@@ -2157,7 +2165,7 @@ resolve_ops (FlatpakTransaction *self,
if (xa_metadata == NULL)
g_message ("Warning: No xa.metadata in local commit %s ref %s", checksum, op->ref);
else
- metadata_bytes = g_bytes_new (xa_metadata, strlen (xa_metadata) + 1);
+ 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);
@@ -2165,7 +2173,8 @@ resolve_ops (FlatpakTransaction *self,
op->installed_size = GUINT64_FROM_BE (installed_size);
old_metadata_bytes = load_deployed_metadata (self, op->ref);
- mark_op_resolved (op, checksum, metadata_bytes, old_metadata_bytes);
+ if (!mark_op_resolved (op, checksum, metadata_bytes, old_metadata_bytes, error))
+ return FALSE;
}
else if (state->collection_id == NULL) /* In the non-p2p case we have all the info available in the summary, so use it */
{
@@ -2200,13 +2209,14 @@ resolve_ops (FlatpakTransaction *self,
g_clear_error (&local_error);
}
else
- metadata_bytes = g_bytes_new (metadata, strlen (metadata) + 1);
+ metadata_bytes = g_bytes_new (metadata, strlen (metadata));
op->installed_size = installed_size;
op->download_size = download_size;
old_metadata_bytes = load_deployed_metadata (self, op->ref);
- mark_op_resolved (op, checksum, metadata_bytes, old_metadata_bytes);
+ if (!mark_op_resolved (op, checksum, metadata_bytes, old_metadata_bytes, error))
+ return FALSE;
}
else
{
diff -urpN flatpak-1.2.3.orig/common/flatpak-utils.c flatpak-1.2.3/common/flatpak-utils.c
--- flatpak-1.2.3.orig/common/flatpak-utils.c 2022-08-30 16:26:04.237655063 -0500
+++ flatpak-1.2.3/common/flatpak-utils.c 2022-08-31 15:29:09.423721916 -0500
@@ -4799,6 +4799,7 @@ flatpak_pull_from_bundle (OstreeRepo *
GCancellable *cancellable,
GError **error)
{
+ gsize metadata_size = 0;
g_autofree char *metadata_contents = NULL;
g_autofree char *to_checksum = NULL;
@@ -4816,6 +4817,8 @@ flatpak_pull_from_bundle (OstreeRepo *
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;
@@ -4885,12 +4888,10 @@ flatpak_pull_from_bundle (OstreeRepo *
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
{
diff -urpN flatpak-1.2.3.orig/tests/Makefile.am.inc flatpak-1.2.3/tests/Makefile.am.inc
--- flatpak-1.2.3.orig/tests/Makefile.am.inc 2019-02-11 07:44:28.000000000 -0600
+++ flatpak-1.2.3/tests/Makefile.am.inc 2022-09-01 09:47:56.657202015 -0500
@@ -127,6 +127,7 @@ TEST_MATRIX_SOURCE = \
tests/test-run.sh{{user+system},{nodeltas+deltas}} \
tests/test-info.sh{user+system} \
tests/test-repo.sh{user+system+collections+collections-server-only} \
+ tests/test-metadata-validation.sh \
tests/test-extensions.sh \
tests/test-bundle.sh{user+system} \
tests/test-oci.sh \
diff -urpN flatpak-1.2.3.orig/tests/Makefile-test-matrix.am.inc flatpak-1.2.3/tests/Makefile-test-matrix.am.inc
--- flatpak-1.2.3.orig/tests/Makefile-test-matrix.am.inc 2019-02-11 07:44:28.000000000 -0600
+++ flatpak-1.2.3/tests/Makefile-test-matrix.am.inc 2022-09-01 09:47:04.841037395 -0500
@@ -21,6 +21,7 @@ TEST_MATRIX_DIST= \
tests/test-config.sh \
tests/test-build-update-repo.sh \
tests/test-http-utils.sh \
+ tests/test-metadata-validation.sh \
tests/test-extensions.sh \
tests/test-oci.sh \
tests/test-unsigned-summaries.sh \
diff -urpN flatpak-1.2.3.orig/tests/test-metadata-validation.sh flatpak-1.2.3/tests/test-metadata-validation.sh
--- flatpak-1.2.3.orig/tests/test-metadata-validation.sh 1969-12-31 18:00:00.000000000 -0600
+++ flatpak-1.2.3/tests/test-metadata-validation.sh 2022-09-01 09:46:07.408854952 -0500
@@ -0,0 +1,158 @@
+#!/bin/bash
+#
+# Copyright (C) 2021 Matthew Leeds <mwleeds@protonmail.com>
+#
+# SPDX-License-Identifier: LGPL-2.0-or-later
+
+set -euo pipefail
+
+. $(dirname $0)/libtest.sh
+
+echo "1..7"
+
+setup_repo
+
+COUNTER=1
+
+create_app () {
+ local OPTIONS="$1"
+ local DIR=`mktemp -d`
+
+ mkdir ${DIR}/files
+ echo $COUNTER > ${DIR}/files/counter
+ let COUNTER=COUNTER+1
+
+ local INVALID=""
+ if [[ $OPTIONS =~ "invalid" ]]; then
+ INVALID=invalidkeyfileline
+ fi
+ cat > ${DIR}/metadata <<EOF
+[Application]
+name=org.test.Malicious
+runtime=org.test.Platform/${ARCH}/master
+$INVALID
+
+[Context]
+EOF
+ if [[ $OPTIONS =~ "mismatch" ]]; then
+ echo -e "filesystems=host;" >> ${DIR}/metadata
+ fi
+ if [[ $OPTIONS =~ "hidden" ]]; then
+ echo -ne "\0" >> ${DIR}/metadata
+ echo -e "\nfilesystems=home;" >> ${DIR}/metadata
+ fi
+ local XA_METADATA=--add-metadata-string=xa.metadata="$(head -n6 ${DIR}/metadata)"$'\n'
+ if [[ $OPTIONS =~ "no-xametadata" ]]; then
+ XA_METADATA="--add-metadata-string=xa.nometadata=1"
+ fi
+ ostree commit --repo=repos/test --branch=app/org.test.Malicious/${ARCH}/master ${FL_GPGARGS} "$XA_METADATA" ${DIR}/
+ if [[ $OPTIONS =~ "no-cache-in-summary" ]]; then
+ ostree --repo=repos/test ${FL_GPGARGS} summary -u
+ # force use of legacy summary format
+ rm -rf repos/test/summary.idx repos/test/summaries
+ else
+ update_repo
+ fi
+ rm -rf ${DIR}
+}
+
+cleanup_repo () {
+ ostree refs --repo=repos/test --delete app/org.test.Malicious/${ARCH}/master
+ update_repo
+}
+
+create_app "hidden"
+
+if ${FLATPAK} ${U} install -y test-repo org.test.Malicious 2>install-error-log; then
+ assert_not_reached "Should not be able to install app with hidden permissions"
+fi
+
+assert_file_has_content install-error-log "not matching expected metadata"
+
+assert_not_has_dir $FL_DIR/app/org.test.Malicious/current/active
+
+cleanup_repo
+
+ok "app with hidden permissions can't be installed (CVE-2021-43860)"
+
+create_app no-xametadata
+
+# The install will fail because the metadata in the summary doesn't match the metadata on the commit
+# The missing xa.metadata in the commit got turned into "" in the xa.cache
+if ${FLATPAK} ${U} install -y test-repo org.test.Malicious 2>install-error-log; then
+ assert_not_reached "Should not be able to install app with missing xa.metadata"
+fi
+
+assert_file_has_content install-error-log "not matching expected metadata"
+
+assert_not_has_dir $FL_DIR/app/org.test.Malicious/current/active
+
+cleanup_repo
+
+ok "app with no xa.metadata can't be installed"
+
+create_app "no-xametadata no-cache-in-summary"
+
+# The install will fail because there's no metadata in the summary or on the commit
+if ${FLATPAK} ${U} install -y test-repo org.test.Malicious 2>install-error-log; then
+ assert_not_reached "Should not be able to install app with missing metadata"
+fi
+assert_file_has_content install-error-log "No xa.metadata in local commit"
+
+assert_not_has_dir $FL_DIR/app/org.test.Malicious/current/active
+
+cleanup_repo
+
+ok "app with no xa.metadata and no metadata in summary can't be installed"
+
+create_app "invalid"
+
+if ${FLATPAK} ${U} install -y test-repo org.test.Malicious 2>install-error-log; then
+ assert_not_reached "Should not be able to install app with invalid metadata"
+fi
+assert_file_has_content install-error-log "Metadata for .* is invalid"
+
+assert_not_has_dir $FL_DIR/app/org.test.Malicious/current/active
+
+cleanup_repo
+
+ok "app with invalid metadata (in summary) can't be installed"
+
+create_app "invalid no-cache-in-summary"
+
+if ${FLATPAK} ${U} install -y test-repo org.test.Malicious 2>install-error-log; then
+ assert_not_reached "Should not be able to install app with invalid metadata"
+fi
+assert_file_has_content install-error-log "Metadata for .* is invalid"
+
+assert_not_has_dir $FL_DIR/app/org.test.Malicious/current/active
+
+cleanup_repo
+
+ok "app with invalid metadata (in commit) can't be installed"
+
+create_app "mismatch no-cache-in-summary"
+
+if ${FLATPAK} ${U} install -y test-repo org.test.Malicious 2>install-error-log; then
+ assert_not_reached "Should not be able to install app with non-matching metadata"
+fi
+assert_file_has_content install-error-log "Commit metadata for .* not matching expected metadata"
+
+assert_not_has_dir $FL_DIR/app/org.test.Malicious/current/active
+
+cleanup_repo
+
+ok "app with mismatched metadata (in commit) can't be installed"
+
+create_app "mismatch"
+
+if ${FLATPAK} ${U} install -y test-repo org.test.Malicious 2>install-error-log; then
+ assert_not_reached "Should not be able to install app with non-matching metadata"
+fi
+assert_file_has_content install-error-log "Commit metadata for .* not matching expected metadata"
+
+assert_not_has_dir $FL_DIR/app/org.test.Malicious/current/active
+
+cleanup_repo
+
+ok "app with mismatched metadata (in summary) can't be installed"
