dir: Use chaseat in extract_extra_data to prevent path traversal

extract_extra_data() had two vulnerabilities:

1. It resolved "files/extra" using g_file_resolve_relative_path()
   which follows symlinks. A crafted OSTree commit with "files" as a
   symlink causes extra-data blobs to be written at the symlink target.
   On system installs this runs as root via the system helper, which
   validates signatures and checksums but not tree structure.

2. It used g_file_get_child(extradir, name) where name comes from
   xa.extra-data-sources in the commit metadata. Names containing ".."
   escape the extra/ directory. This is exploitable through the normal
   build flow: flatpak build-export rejects "/" but not "..".

Replace GFile path operations with fd-relative operations: open
"files" with glnx_chaseat using GLNX_CHASE_RESOLVE_NO_SYMLINKS,
create "extra" with glnx_chase_and_mkdirat using
GLNX_CHASE_RESOLVE_BENEATH, validate extra-data names against ".",
"..", and "/", and write with glnx_file_replace_contents_at anchored
to the extra directory fd.

[smcv: Open checkoutdir_dfd before trying to open its files subdir]
Co-authored-by: Simon McVittie <smcv@collabora.com>
Resolves: https://github.com/flatpak/flatpak/security/advisories/GHSA-fqx6-vh4p-42cg
This commit is contained in:
Sebastian Wick committed 2026-08-11 01:22:28 +02:00
1 parent f635826dc7
commit 70fdf20886
1 file changed
+31 -16
+31 -16
View File
@@ -9093,7 +9093,7 @@ out:
static gboolean
extract_extra_data (FlatpakDir *self,
const char *checksum,
GFile *extradir,
int app_files_dfd,
gboolean *created_extra_data,
GCancellable *cancellable,
GError **error)
@@ -9102,6 +9102,7 @@ extract_extra_data (FlatpakDir *self,
g_autoptr(GVariant) extra_data = NULL;
g_autoptr(GVariant) extra_data_sources = NULL;
g_autoptr(GError) local_error = NULL;
glnx_autofd int extra_dfd = -1;
gsize i, n_extra_data = 0;
gsize n_extra_data_sources;
@@ -9124,7 +9125,7 @@ extract_extra_data (FlatpakDir *self,
if (n_extra_data_sources == 0)
return TRUE;
g_info ("extracting extra data to %s", flatpak_file_get_path_cached (extradir));
g_info ("extracting extra data");
if (!ostree_repo_read_commit_detached_metadata (self->repo, checksum, &detached_metadata,
cancellable, error))
@@ -9145,7 +9146,10 @@ extract_extra_data (FlatpakDir *self,
if (n_extra_data < n_extra_data_sources)
return flatpak_fail_error (error, FLATPAK_ERROR_INVALID_DATA, _("Extra data missing in detached metadata"));
if (!flatpak_mkdir_p (extradir, cancellable, error))
extra_dfd = glnx_chase_and_mkdirat (app_files_dfd, "extra",
GLNX_CHASE_RESOLVE_BENEATH,
0777, error);
if (extra_dfd < 0)
{
g_prefix_error (error, _("While creating extradir: "));
return FALSE;
@@ -9180,7 +9184,6 @@ extract_extra_data (FlatpakDir *self,
for (j = 0; j < n_extra_data; j++)
{
g_autoptr(GVariant) content = NULL;
g_autoptr(GFile) dest = NULL;
g_autofree char *sha256 = NULL;
const char *extra_data_name = NULL;
const guchar *data;
@@ -9193,6 +9196,13 @@ extract_extra_data (FlatpakDir *self,
if (strcmp (extra_data_source_name, extra_data_name) != 0)
continue;
if (extra_data_name[0] == '\0' ||
strcmp (extra_data_name, ".") == 0 ||
strcmp (extra_data_name, "..") == 0 ||
strchr (extra_data_name, '/') != NULL)
return flatpak_fail_error (error, FLATPAK_ERROR_INVALID_DATA,
_("Invalid extra data filename '%s'"), extra_data_name);
data = g_variant_get_data (content);
len = g_variant_get_size (content);
@@ -9203,12 +9213,10 @@ extract_extra_data (FlatpakDir *self,
if (strcmp (sha256, extra_data_sha256) != 0)
return flatpak_fail_error (error, FLATPAK_ERROR_INVALID_DATA, _("Invalid checksum for extra data"));
dest = g_file_get_child (extradir, extra_data_name);
if (!g_file_replace_contents (dest,
g_variant_get_data (content),
g_variant_get_size (content),
NULL, FALSE, G_FILE_CREATE_REPLACE_DESTINATION,
NULL, cancellable, error))
if (!glnx_file_replace_contents_at (extra_dfd, extra_data_name,
data, len,
GLNX_FILE_REPLACE_NODATASYNC,
cancellable, error))
{
g_prefix_error (error, _("While writing extra data file '%s': "), extra_data_name);
return FALSE;
@@ -9663,6 +9671,7 @@ flatpak_dir_deploy (FlatpakDir *self,
OstreeRepoCheckoutAtOptions options = { 0, };
const char *checksum;
glnx_autofd int checkoutdir_dfd = -1;
glnx_autofd int app_files_dfd = -1;
const char *xa_ref = NULL;
g_autofree char *checkout_basename = NULL;
gboolean created_extra_data = FALSE;
@@ -9819,15 +9828,24 @@ flatpak_dir_deploy (FlatpakDir *self,
}
}
if (!glnx_opendirat (deploy_base_dfd, checkoutdir_basename, FALSE, &checkoutdir_dfd, error))
return FALSE;
/* Extract any extra data */
extradir = g_file_resolve_relative_path (checkoutdir, "files/extra");
if (!flatpak_rm_rf (extradir, cancellable, error))
app_files_dfd = glnx_chaseat (checkoutdir_dfd, "files",
GLNX_CHASE_RESOLVE_NO_SYMLINKS |
GLNX_CHASE_MUST_BE_DIRECTORY,
error);
if (app_files_dfd < 0)
return FALSE;
if (!glnx_shutil_rm_rf_at (app_files_dfd, "extra", cancellable, error))
{
g_prefix_error (error, _("While trying to remove existing extra dir: "));
return FALSE;
}
if (!extract_extra_data (self, checksum, extradir, &created_extra_data, cancellable, error))
if (!extract_extra_data (self, checksum, app_files_dfd, &created_extra_data, cancellable, error))
return FALSE;
if (created_extra_data)
@@ -10036,9 +10054,6 @@ flatpak_dir_deploy (FlatpakDir *self,
if (!flatpak_bytes_save (deploy_data_file, deploy_data, cancellable, error))
return FALSE;
if (!glnx_opendirat (deploy_base_dfd, checkoutdir_basename, TRUE, &checkoutdir_dfd, error))
return FALSE;
if (syncfs (checkoutdir_dfd) != 0)
{
glnx_set_error_from_errno (error);