diff --git a/backport-0001-CVE-2021-28650.patch b/backport-0001-CVE-2021-28650.patch new file mode 100644 index 0000000000000000000000000000000000000000..63093a4cf6407b8edd0ebe202abe6d46a43537f3 --- /dev/null +++ b/backport-0001-CVE-2021-28650.patch @@ -0,0 +1,72 @@ +From 88e21e8aa2841216fa1d7fba617a8692912af51e 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. +Conflict:NA +Reference:https://gitlab.gnome.org/GNOME/gnome-autoar/-/merge_requests/15/diffs?commit_id=88e21e8aa2841216fa1d7fba617a8692912af51e +--- + 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.19.1 + diff --git a/backport-0002-CVE-2021-28650.patch b/backport-0002-CVE-2021-28650.patch new file mode 100644 index 0000000000000000000000000000000000000000..80557aaa3fb76e3b3eb42b8abc13788be3575848 --- /dev/null +++ b/backport-0002-CVE-2021-28650.patch @@ -0,0 +1,129 @@ +From 8109c368c6cfdb593faaf698c2bf5da32bb1ace4 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 +Conflict:NA +Reference:https://gitlab.gnome.org/GNOME/gnome-autoar/-/merge_requests/15/diffs?commit_id=8109c368c6cfdb593faaf698c2bf5da32bb1ace4 +--- + gnome-autoar/autoar-extractor.c | 60 +++++++++++++++++++++++++-------- + 1 file changed, 46 insertions(+), 14 deletions(-) + +diff --git a/gnome-autoar/autoar-extractor.c b/gnome-autoar/autoar-extractor.c +index 376c864..d84a726 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_NONE, + 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); +@@ -1822,12 +1837,27 @@ autoar_extractor_step_extract (AutoarExtractor *self) { + autoar_extractor_do_sanitize_pathname (self, hardlink); + } + +- /* 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 +1885,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.19.1 + diff --git a/backport-CVE-2020-36241.patch b/backport-CVE-2020-36241.patch deleted file mode 100644 index eb968e3050b044b4dbe4714e719b9caaa8920170..0000000000000000000000000000000000000000 --- a/backport-CVE-2020-36241.patch +++ /dev/null @@ -1,101 +0,0 @@ -diff -Naur gnome-autoar-0.2.4.old/gnome-autoar/autoar-extractor.c gnome-autoar-0.2.4/gnome-autoar/autoar-extractor.c ---- gnome-autoar-0.2.4.old/gnome-autoar/autoar-extractor.c 2019-03-09 00:53:15.000000000 +0800 -+++ gnome-autoar-0.2.4/gnome-autoar/autoar-extractor.c 2021-03-18 22:01:20.838393707 +0800 -@@ -881,32 +881,67 @@ - return prefix; - } - -+static gboolean -+is_valid_filename (GFile *file, GFile *destination) -+{ -+ g_autoptr (GFile) parent = NULL; -+ g_autoptr (GFileInfo) info = NULL; -+ -+ if (g_file_equal (file, destination)) -+ return TRUE; -+ -+ if (!g_file_has_prefix (file, destination)) -+ return FALSE; -+ -+ /* Resolve symbolic link ancestors to confirm file is actually inside destination. */ -+ parent = g_file_get_parent (file); -+ info = g_file_query_info (parent, -+ G_FILE_ATTRIBUTE_STANDARD_IS_SYMLINK "," -+ G_FILE_ATTRIBUTE_STANDARD_SYMLINK_TARGET, -+ G_FILE_QUERY_INFO_NOFOLLOW_SYMLINKS, -+ NULL, -+ NULL); -+ if (info == NULL) -+ return FALSE; -+ -+ if (g_file_info_get_is_symlink (info)) { -+ g_autoptr (GFile) cwd = NULL; -+ const gchar *target; -+ -+ target = g_file_info_get_symlink_target (info); -+ if (g_path_is_absolute (target)) -+ return FALSE; -+ -+ cwd = g_file_get_parent (parent); -+ g_object_unref (parent); -+ parent = g_file_resolve_relative_path (cwd, target); -+ } -+ -+ /* Climb up the path to resolve every symbolic link ancestor found */ -+ return is_valid_filename (parent, destination); -+} -+ - static GFile* - autoar_extractor_do_sanitize_pathname (AutoarExtractor *self, - const char *pathname_bytes) - { - GFile *extracted_filename; - gboolean valid_filename; -- g_autofree char *sanitized_pathname; -+ g_autofree char *sanitized_pathname = NULL; - g_autofree char *utf8_pathname; - - utf8_pathname = autoar_common_get_utf8_pathname (pathname_bytes); - extracted_filename = g_file_get_child (self->destination_dir, - utf8_pathname ? utf8_pathname : pathname_bytes); - -- valid_filename = -- g_file_equal (extracted_filename, self->destination_dir) || -- g_file_has_prefix (extracted_filename, self->destination_dir); -- -+ valid_filename = is_valid_filename (extracted_filename, self->destination_dir); - if (!valid_filename) { -- g_autofree char *basename; -- -- basename = g_file_get_basename (extracted_filename); -- -+ g_warning ("autoar_extractor_do_sanitize_pathname: %s is outside of the destination dir", -+ g_file_peek_path (extracted_filename)); -+ - g_object_unref (extracted_filename); - -- extracted_filename = g_file_get_child (self->destination_dir, -- basename); -+ return NULL; - } - - if (self->prefix != NULL && self->new_prefix != NULL) { -@@ -1862,10 +1897,17 @@ - - extracted_filename = - autoar_extractor_do_sanitize_pathname (self, pathname); -- -+ if (extracted_filename == NULL) { -+ archive_read_data_skip (a); -+ continue; -+ } - if (hardlink != NULL) { - hardlink_filename = - autoar_extractor_do_sanitize_pathname (self, hardlink); -+ if (hardlink_filename == NULL) { -+ archive_read_data_skip (a); -+ continue; -+ } - } - - /* Attempt to solve any name conflict before doing any operations */ diff --git a/gnome-autoar.spec b/gnome-autoar.spec index c960520ce234e72ac5df18393717f7d4765ee72b..69d8cf84b1c4ed84803de6656068be4658fbef9f 100644 --- a/gnome-autoar.spec +++ b/gnome-autoar.spec @@ -1,12 +1,13 @@ Name: gnome-autoar Version: 0.2.3 -Release: 5 +Release: 6 Summary: Creating and extracting archives 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 -Patch6000: backport-CVE-2020-36241.patch +Patch6000: backport-0001-CVE-2021-28650.patch +Patch6001: backport-0002-CVE-2021-28650.patch BuildRequires: gcc vala pkgconfig(gio-2.0) pkgconfig(glib-2.0) pkgconfig(gobject-2.0) gdb BuildRequires: pkgconfig(gobject-introspection-1.0) pkgconfig(gtk+-3.0) pkgconfig(libarchive) @@ -50,6 +51,12 @@ make check %{_datadir}/vala/vapi/*.vapi %changelog +* Wed Mar 31 2021 Dehui Fan - 0.2.3-6 +- Type: CVE +- ID: CVE-2021-28650 +- SUG: NA +- DESC: fix CVE-2021-28650, remove CVE-2020-36241 + * Fri Mar 19 2021 Dehui Fan - 0.2.3-5 - Type: CVE - ID: CVE-2020-36241