Attempt n.2 :
Created a function that walks the whole path. If anything not-directory is
found, return an error. If the path is not fully created, we consider that
a success and let cpio decides when time has come.
Files will be skipped if no-absolute-path is set and error is return.
On Wed, Jun 7, 2017 at 10:46 AM, Pavel Raiskup <prais...@redhat.com> wrote:
> On Wednesday, June 7, 2017 10:07:21 AM CEST Cedric Buissart wrote:
> > > In other words and IMO, if we were about to fix this issue - we should
> only
> > > refuse to extract files through symlinks.
> >
> > Through any symlinks, or only those created by the archive itself ?
>
> Remembering the extracted links might be expensive, and with
> --no-absolute-filenames we want to stay in CWD anyway - no matter how the
> links
> in CWD were created.
>
> > The latter might look less restrictive, but what happens if a local
> > attacker is able to create a symlink. Is it something that should be
> > considered ?
>
> Usually user should avoid races manually when running archiver:
> https://www.gnu.org/software/tar/manual/html_node/Race-conditions.html
based on the above, I did not try to avoid races.
>
>
> Pavel
>
>
>
>
--
Cedric Buissart,
Product Security
commit 3fa889b71dd543b70c511d9b84894bd096061d51
Author: Cedric Buissart <cedric.buiss...@gmail.com>
Date: Thu Jun 15 21:00:43 2017 +0200
Prevent symlink follow when no-absolute-path
diff --git a/src/copyin.c b/src/copyin.c
index 38d809f..809b9e9 100644
--- a/src/copyin.c
+++ b/src/copyin.c
@@ -1465,6 +1465,16 @@ process_copy_in ()
}
}
+
+#ifdef CP_IFLNK
+ /* no_abs_paths_flag prevents us to follow symlink */
+ if (no_abs_paths_flag && existing_path_has_symlink(file_hdr.c_name) == -1) {
+ error (0, 0, _("%s not created: symlink in its path"), file_hdr.c_name);
+ tape_toss_input (in_file_des, file_hdr.c_filesize);
+ tape_skip_padding (in_file_des, file_hdr.c_filesize);
+ continue;
+ }
+#endif
copyin_file(&file_hdr, in_file_des);
if (verbose_flag)
diff --git a/src/util.c b/src/util.c
index 18b3e42..dd04edf 100644
--- a/src/util.c
+++ b/src/util.c
@@ -1627,3 +1627,45 @@ change_dir ()
_("cannot change to directory `%s'"), change_directory_option);
}
}
+
+#ifdef CP_IFLNK
+/* existing_path_has_symlink will walk the path as far as possible.
+ * returns -1 if unexpected error or if any non-directory is found on the way
+ * (with the exception of the filename)
+ * returns 0 otherwise
+ * Note: if part of the path is missing, it's ok, cpio might be allowed to create it
+ */
+int existing_path_has_symlink(char* path) {
+ int ret = 0;
+ struct stat stat;
+ char *next, *dup;
+ dup = malloc((strlen(path)+1) * sizeof(char));
+ if (! dup)
+ return -1;
+
+ next = path;
+ while (1) {
+ next = strchr(next, '/');
+ if (! next) {
+ break;
+ }
+ strncpy(dup, path, next - path);
+ dup[next - path] = '\0';
+ if ( lstat(dup, &stat) != 0) {
+ /* only ENOENT is expected here (we let cpio create path if required */
+ if ( errno != ENOENT )
+ ret = -1;
+ break;
+ }
+
+ if ( ! S_ISDIR(stat.st_mode) ) {
+ ret = -1;
+ break;
+ }
+ next++;
+ }
+
+ free(dup);
+ return ret;
+}
+#endif
