mirror of
https://github.com/flatpak/flatpak.git
synced 2026-05-11 09:30:09 -04:00
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 <ryan.gonzalez@collabora.com> (cherry picked from commit c9c3a667c09a846c0b230cf1cc8ed330028aa03c)
This commit is contained in:
committed by
Alexander Larsson
parent
9fded3abe4
commit
5daba75daf
@@ -1792,19 +1792,29 @@ static gboolean
|
||||
validate_commit_metadata (GVariant *commit_data,
|
||||
const char *ref,
|
||||
const char *required_metadata,
|
||||
gsize required_metadata_size,
|
||||
gboolean require_xa_metadata,
|
||||
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))
|
||||
(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);
|
||||
@@ -3513,6 +3523,7 @@ upgrade_deploy_data (GBytes *deploy_data,
|
||||
g_autoptr(GKeyFile) keyfile = NULL;
|
||||
g_autoptr(GFile) metadata_file = NULL;
|
||||
g_autofree char *metadata_contents = NULL;
|
||||
gsize metadata_size = 0;
|
||||
g_autofree char *id = flatpak_decomposed_dup_id (ref);
|
||||
|
||||
/* Add fields from commit metadata to deploy */
|
||||
@@ -3526,9 +3537,9 @@ upgrade_deploy_data (GBytes *deploy_data,
|
||||
keyfile = g_key_file_new ();
|
||||
metadata_file = g_file_resolve_relative_path (deploy_dir, "metadata");
|
||||
if (!g_file_load_contents (metadata_file, cancellable,
|
||||
&metadata_contents, NULL, NULL, error))
|
||||
&metadata_contents, &metadata_size, NULL, error))
|
||||
return NULL;
|
||||
if (!g_key_file_load_from_data (keyfile, metadata_contents, -1, 0, error))
|
||||
if (!g_key_file_load_from_data (keyfile, metadata_contents, metadata_size, 0, error))
|
||||
return NULL;
|
||||
add_metadata_to_deploy_data (&metadata_dict, keyfile);
|
||||
|
||||
@@ -5831,8 +5842,13 @@ flatpak_dir_pull (FlatpakDir *self,
|
||||
{
|
||||
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),
|
||||
TRUE,
|
||||
error))
|
||||
goto out;
|
||||
}
|
||||
|
||||
if (!flatpak_dir_pull_extra_data (self, repo,
|
||||
@@ -8153,6 +8169,7 @@ flatpak_dir_deploy (FlatpakDir *self,
|
||||
g_auto(GLnxLockFile) lock = { 0, };
|
||||
g_autoptr(GFile) metadata_file = NULL;
|
||||
g_autofree char *metadata_contents = NULL;
|
||||
gsize metadata_size = 0;
|
||||
gboolean is_oci;
|
||||
const char *flatpak;
|
||||
|
||||
@@ -8363,11 +8380,12 @@ flatpak_dir_deploy (FlatpakDir *self,
|
||||
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;
|
||||
|
||||
@@ -8383,7 +8401,7 @@ flatpak_dir_deploy (FlatpakDir *self,
|
||||
*/
|
||||
is_oci = flatpak_dir_get_remote_oci (self, origin);
|
||||
if (!validate_commit_metadata (commit_data, flatpak_decomposed_get_ref (ref),
|
||||
metadata_contents, !is_oci, error))
|
||||
metadata_contents, metadata_size, !is_oci, error))
|
||||
return FALSE;
|
||||
|
||||
dotref = g_file_resolve_relative_path (checkoutdir, "files/.ref");
|
||||
|
||||
@@ -2539,7 +2539,7 @@ flatpak_transaction_add_ref (FlatpakTransaction *self,
|
||||
return FALSE;
|
||||
|
||||
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;
|
||||
}
|
||||
@@ -2950,7 +2950,7 @@ load_deployed_metadata (FlatpakTransaction *self, FlatpakDecomposed *ref, char *
|
||||
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
|
||||
@@ -3051,7 +3051,7 @@ resolve_op_from_commit (FlatpakTransaction *self,
|
||||
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) + 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);
|
||||
@@ -3091,7 +3091,7 @@ try_resolve_op_from_metadata (FlatpakTransaction *self,
|
||||
&download_size, &installed_size, &metadata, NULL))
|
||||
return FALSE;
|
||||
|
||||
metadata_bytes = g_bytes_new (metadata, strlen (metadata) + 1);
|
||||
metadata_bytes = g_bytes_new (metadata, strlen (metadata));
|
||||
|
||||
if (flatpak_remote_state_lookup_ref (state, flatpak_decomposed_get_ref (op->ref),
|
||||
NULL, NULL, &info, NULL, NULL))
|
||||
|
||||
@@ -6662,6 +6662,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;
|
||||
g_autoptr(GFile) root = NULL;
|
||||
@@ -6678,6 +6679,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;
|
||||
@@ -6747,12 +6750,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
|
||||
{
|
||||
|
||||
Reference in New Issue
Block a user