diff --git a/SOURCES/extractor-Detect-conflict-also-for-directories.patch b/SOURCES/extractor-Detect-conflict-also-for-directories.patch new file mode 100644 index 0000000..fc24527 --- /dev/null +++ b/SOURCES/extractor-Detect-conflict-also-for-directories.patch @@ -0,0 +1,70 @@ +From 2c7a42b63913c05326cb66253960517ea0343c6a Mon Sep 17 00:00:00 2001 +From: Ondrej Holy +Date: Thu, 25 Feb 2021 14:10:26 +0100 +Subject: [PATCH] extractor: Detect conflict also for directories + +Current logic doesn't detect conflics when extracting directory. This +is ok, but only for the case when the conflic is caused by directory. +Otherwise, the conflic should be detected and AutoarExtractor should +try to delete the file before creating new directory. +--- + gnome-autoar/autoar-extractor.c | 27 ++++++++------------------- + 1 file changed, 8 insertions(+), 19 deletions(-) + +diff --git a/gnome-autoar/autoar-extractor.c b/gnome-autoar/autoar-extractor.c +index f1f49cf..376c864 100644 +--- a/gnome-autoar/autoar-extractor.c ++++ b/gnome-autoar/autoar-extractor.c +@@ -897,7 +897,6 @@ autoar_extractor_check_file_conflict (GFile *file, + mode_t extracted_filetype) + { + GFileType file_type; +- gboolean conflict = FALSE; + + file_type = g_file_query_file_type (file, + G_FILE_QUERY_INFO_NONE, +@@ -907,26 +906,13 @@ autoar_extractor_check_file_conflict (GFile *file, + return FALSE; + } + +- switch (extracted_filetype) { +- case AE_IFDIR: +- break; +- case AE_IFREG: +- case AE_IFLNK: +-#if defined HAVE_MKFIFO || defined HAVE_MKNOD +- case AE_IFIFO: +-#endif +-#ifdef HAVE_MKNOD +- case AE_IFSOCK: +- case AE_IFBLK: +- case AE_IFCHR: +-#endif +- conflict = TRUE; +- break; +- default: +- break; ++ /* It is not problem if the directory already exists */ ++ if (file_type == G_FILE_TYPE_DIRECTORY && ++ extracted_filetype == AE_IFDIR) { ++ return FALSE; + } + +- return conflict; ++ return TRUE; + } + + static void +@@ -1850,6 +1836,9 @@ autoar_extractor_step_extract (AutoarExtractor *self) { + case AUTOAR_CONFLICT_OVERWRITE: + break; + case AUTOAR_CONFLICT_CHANGE_DESTINATION: ++ /* FIXME: If the destination is changed for directory, it should be ++ * changed also for its children... ++ */ + g_assert_nonnull (new_extracted_filename); + g_clear_object (&extracted_filename); + extracted_filename = new_extracted_filename; +-- +2.31.1 + diff --git a/SOURCES/extractor-Do-not-allow-symlink-in-parents.patch b/SOURCES/extractor-Do-not-allow-symlink-in-parents.patch new file mode 100644 index 0000000..facc6df --- /dev/null +++ b/SOURCES/extractor-Do-not-allow-symlink-in-parents.patch @@ -0,0 +1,126 @@ +From 3e7b4aca4b0afe9fb1b1160bd26f791d7a636980 Mon Sep 17 00:00:00 2001 +From: Ondrej Holy +Date: Mon, 1 Mar 2021 17:16:27 +0100 +Subject: [PATCH] extractor: Do not allow symlink in parents + +Currently, it is still possible that some files are extracted outside of +the destination dir in case of malicious archives. The checks from commit +adb067e6 can be still bypassed in certain cases. See GNOME/file-roller#108 +for more details. After some investigation, I am convinced that it would be +best to simply disallow symlinks in parents. For example, `tar` fails to +extract such files with the `ENOTDIR` error. Let's do the same here. + +Fixes: https://gitlab.gnome.org/GNOME/gnome-autoar/-/issues/12 +--- + gnome-autoar/autoar-extractor.c | 59 +++++++++++++++++++++++++-------- + 1 file changed, 46 insertions(+), 13 deletions(-) + +diff --git a/gnome-autoar/autoar-extractor.c b/gnome-autoar/autoar-extractor.c +index ce6e6e9..79a7278 100644 +--- a/gnome-autoar/autoar-extractor.c ++++ b/gnome-autoar/autoar-extractor.c +@@ -892,27 +892,42 @@ autoar_extractor_do_sanitize_pathname (AutoarExtractor *self, + return extracted_filename; + } + +-static gboolean +-autoar_extractor_check_file_conflict (GFile *file, ++/* The function checks @file for conflicts with already existing files on the ++ * disk. It also recursively checks parents of @file to be sure it is directory. ++ * It doesn't follow symlinks, so symlinks in parents are also considered as ++ * conflicts even though they point to directory. It returns #GFile object for ++ * the file, which cause the conflict (so @file, or some of its parents). If ++ * there aren't any conflicts, NULL is returned. ++ */ ++static GFile * ++autoar_extractor_check_file_conflict (AutoarExtractor *self, ++ GFile *file, + mode_t extracted_filetype) + { + GFileType file_type; ++ g_autoptr (GFile) parent = NULL; + + file_type = g_file_query_file_type (file, + G_FILE_QUERY_INFO_NOFOLLOW_SYMLINKS, + NULL); +- /* If there is no file with the given name, there will be no conflict */ +- if (file_type == G_FILE_TYPE_UNKNOWN) { +- return FALSE; ++ ++ /* It is a conflict if the file already exists with an exception for already ++ * existing directories. ++ */ ++ if (file_type != G_FILE_TYPE_UNKNOWN && ++ (file_type != G_FILE_TYPE_DIRECTORY || ++ extracted_filetype != AE_IFDIR)) { ++ return g_object_ref (file); + } + +- /* It is not problem if the directory already exists */ +- if (file_type == G_FILE_TYPE_DIRECTORY && +- extracted_filetype == AE_IFDIR) { +- return FALSE; ++ if ((self->new_prefix && g_file_equal (self->new_prefix, file)) || ++ (!self->new_prefix && g_file_equal (self->destination_dir, file))) { ++ return NULL; + } + +- return TRUE; ++ /* Check also parents for conflict to be sure it is directory. */ ++ parent = g_file_get_parent (file); ++ return autoar_extractor_check_file_conflict (self, parent, AE_IFDIR); + } + + static void +@@ -1804,7 +1819,7 @@ autoar_extractor_step_extract (AutoarExtractor *self) { + g_autoptr (GFile) extracted_filename = NULL; + g_autoptr (GFile) hardlink_filename = NULL; + AutoarConflictAction action; +- gboolean file_conflict; ++ g_autoptr (GFile) file_conflict = NULL; + + if (g_cancellable_is_cancelled (self->cancellable)) { + archive_read_free (a); +@@ -1823,11 +1838,27 @@ autoar_extractor_step_extract (AutoarExtractor *self) { + } + + /* Attempt to solve any name conflict before doing any operations */ +- file_conflict = autoar_extractor_check_file_conflict (extracted_filename, ++ file_conflict = autoar_extractor_check_file_conflict (self, ++ extracted_filename, + archive_entry_filetype (entry)); + while (file_conflict) { + GFile *new_extracted_filename = NULL; + ++ /* Do not try to solve any conflicts in parents for now. Especially ++ * symlinks in parents are dangerous as it can easily happen that files ++ * are written outside of the destination. The tar cmd fails to extract ++ * such archives with ENOTDIR. Let's do the same here. This is most ++ * probably malicious, or corrupted archive if the conflict was caused ++ * only by files from the archive... ++ */ ++ if (!g_file_equal (file_conflict, extracted_filename)) { ++ self->error = g_error_new (G_IO_ERROR, ++ G_IO_ERROR_NOT_DIRECTORY, ++ "The file is not a directory"); ++ archive_read_free (a); ++ return; ++ } ++ + action = autoar_extractor_signal_conflict (self, + extracted_filename, + &new_extracted_filename); +@@ -1855,7 +1886,9 @@ autoar_extractor_step_extract (AutoarExtractor *self) { + break; + } + +- file_conflict = autoar_extractor_check_file_conflict (extracted_filename, ++ g_clear_object (&file_conflict); ++ file_conflict = autoar_extractor_check_file_conflict (self, ++ extracted_filename, + archive_entry_filetype (entry)); + } + +-- +2.31.1 + diff --git a/SOURCES/extractor-Do-not-follow-symlinks-when-detecting-conf.patch b/SOURCES/extractor-Do-not-follow-symlinks-when-detecting-conf.patch new file mode 100644 index 0000000..3464e28 --- /dev/null +++ b/SOURCES/extractor-Do-not-follow-symlinks-when-detecting-conf.patch @@ -0,0 +1,27 @@ +From c726022a46d780c0cf305788b8126f45704ef462 Mon Sep 17 00:00:00 2001 +From: Ondrej Holy +Date: Mon, 1 Mar 2021 10:13:17 +0100 +Subject: [PATCH] extractor: Do not follow symlinks when detecting conflicts + +Currently, symlinks are followed when detecting conflicts. But this +is not desired as the original file caused the conflict, not its target. +--- + gnome-autoar/autoar-extractor.c | 2 +- + 1 file changed, 1 insertion(+), 1 deletion(-) + +diff --git a/gnome-autoar/autoar-extractor.c b/gnome-autoar/autoar-extractor.c +index 376c864..ce6e6e9 100644 +--- a/gnome-autoar/autoar-extractor.c ++++ b/gnome-autoar/autoar-extractor.c +@@ -899,7 +899,7 @@ autoar_extractor_check_file_conflict (GFile *file, + GFileType file_type; + + file_type = g_file_query_file_type (file, +- G_FILE_QUERY_INFO_NONE, ++ G_FILE_QUERY_INFO_NOFOLLOW_SYMLINKS, + NULL); + /* If there is no file with the given name, there will be no conflict */ + if (file_type == G_FILE_TYPE_UNKNOWN) { +-- +2.31.1 + diff --git a/SPECS/gnome-autoar.spec b/SPECS/gnome-autoar.spec index d1be86c..83cb062 100644 --- a/SPECS/gnome-autoar.spec +++ b/SPECS/gnome-autoar.spec @@ -1,12 +1,16 @@ Name: gnome-autoar Version: 0.2.3 -Release: 1%{?dist} +Release: 2%{?dist} Summary: Archive library License: LGPLv2+ URL: https://git.gnome.org/browse/gnome-autoar Source0: https://download.gnome.org/sources/gnome-autoar/0.2/gnome-autoar-%{version}.tar.xz +Patch0: extractor-Detect-conflict-also-for-directories.patch +Patch1: extractor-Do-not-follow-symlinks-when-detecting-conf.patch +Patch2: extractor-Do-not-allow-symlink-in-parents.patch + BuildRequires: gcc BuildRequires: pkgconfig(gio-2.0) BuildRequires: pkgconfig(glib-2.0) @@ -30,7 +34,7 @@ developing applications that use %{name}. %prep -%autosetup +%autosetup -p1 %build @@ -71,6 +75,9 @@ make check %changelog +* Thu Apr 29 2021 Ondrej Holy - 0.2.3-2 +- CVE-2020-36241, CVE-2021-28650: Do not allow symlink in parents (rhbz#1928701) + * Sat Mar 03 2018 Kalev Lember - 0.2.3-1 - Update to 0.2.3 - Drop ldconfig scriptlets