Re: [Bug-tar] [PATCH] Option to treat tarbombs as errors
On Wed, 27 Nov 2013 18:58:03 -0800
Connor Behan connor.be...@gmail.com wrote:
On 27/11/13 08:25 AM, Joshua Hudson wrote:
It is my opinion this patch is incorrect in design.
If I were to arrange for the first file to be .bashrc, you would get the
error, but the damage would already be done.
Good point. The patch prevents the clutter of a tarbomb but it wouldn't
be able to save this first file.
What would be immediately beneficial is a patch that provides option if
given tar -xf filename.tar*, ensuring
that all paths begin with filename/, and prepending filename/ if they
don't. Do mkdir filename first for the obvious reason.
You might be interested in and earlier version of the patch:
http://lists.gnu.org/archive/html/bug-tar/2013-08/msg00031.html
Paul said a simpler patch that just causes an error would be preferable
but so far neither version has been accepted. It all depends on what
Sergey says if / when he looks at this. I'll post an updated version
that prepends filename/ and then try to ping the maintainers.
Connor,
prepending the package name (if the directory does not yet exist) would also
cope with
archives containing top level . entries. Such packages are common because
some developers
have the habit to create new releases by issuing a command similar to the
following:
$ cd ~/package-xyz-1.0 tar -cf /tmp/package-xyz-1.0.tar .
Now, if you're using a source distribution, such an archive may get
automatically extracted within
a privileged process. In the particular case I observed some time ago,
unpacking the sources was
roughly done like
# P=package-xyz-1.0 mkdir -p /tmp/${P}/work tar -C /tmp/${P}/work
-xf /tmp/${P}.tar
The next step was to set the access rights *within* the newly created work
directory. But because
the tar archive contained
drwxr-xr-x 7121/7121 0 2013-11-28 14:12 ./
-rw-r--r-- 7121/7121 0 2013-11-28 14:12 ./some-file.txt
with the ./ entry denoting the current directory, the newly created work
directory was
still owned by the package author, and not by the privileged user who created
the work directory:
$ ls -ld /tmp/package-xyz-1.0/work
drwxr-xr-x 2 7121 7121 4096 28. Nov 14:12 /tmp/package-xyz-1.0/work
While tar behaved correctly here, this certainly was a defect of the
distribution (Gentoo
in this case). Yet it can also be seen as unexpected behaviour.
Best regards
Hugo Mildenberger
--
Re: [Bug-tar] [PATCH] Option to treat tarbombs as errors
It is my opinion this patch is incorrect in design. If I were to arrange for the first file to be .bashrc, you would get the error, but the damage would already be done. What would be immediately beneficial is a patch that provides option if given tar -xf filename.tar*, ensuring that all paths begin with filename/, and prepending filename/ if they don't. Do mkdir filename first for the obvious reason.
Re: [Bug-tar] [PATCH] Option to treat tarbombs as errors
On 27/11/13 08:25 AM, Joshua Hudson wrote: It is my opinion this patch is incorrect in design. If I were to arrange for the first file to be .bashrc, you would get the error, but the damage would already be done. Good point. The patch prevents the clutter of a tarbomb but it wouldn't be able to save this first file. What would be immediately beneficial is a patch that provides option if given tar -xf filename.tar*, ensuring that all paths begin with filename/, and prepending filename/ if they don't. Do mkdir filename first for the obvious reason. You might be interested in and earlier version of the patch: http://lists.gnu.org/archive/html/bug-tar/2013-08/msg00031.html Paul said a simpler patch that just causes an error would be preferable but so far neither version has been accepted. It all depends on what Sergey says if / when he looks at this. I'll post an updated version that prepends filename/ and then try to ping the maintainers. signature.asc Description: OpenPGP digital signature
[Bug-tar] [PATCH] Option to treat tarbombs as errors
* src/common.h (one_top_level_option): New global.
* src/extract.c (extract_archive): If one_top_level_option is set, check
for whether the archive tries to create two file trees.
* src/tar.c (ONE_TOP_LEVEL_OPTION): New constant.
(options): New option --one-top-level.
(parse_opt): Handle this option.
(decode_options): Make it conflict with --absolute-names.
* NEWS: Update.
* doc/tar.texi: Document new option.
---
NEWS | 10 ++
doc/tar.texi | 9 +
src/common.h | 3 +++
src/extract.c | 43 +++
src/tar.c | 10 ++
5 files changed, 75 insertions(+)
diff --git a/NEWS b/NEWS
index 1a264b0..383d711 100644
--- a/NEWS
+++ b/NEWS
@@ -1,6 +1,16 @@
GNU tar NEWS - User visible changes. 2013-11-17
Please send GNU tar bug reports to bug-tar@gnu.org
+version 1.27.90 (Git)
+
+* The --one-top-level option.
+
+This new command line option tells tar that the working directory
+(or the one passed to -C) should not be populated with more than one
+name directly under it. A fatal error is thrown if, after name
+transformations, a second member that would be extracted to this location
+is found.
+
version 1.27.1 - Sergey Poznyakoff, 2013-11-17
diff --git a/doc/tar.texi b/doc/tar.texi
index 9fde5a0..86d7064 100644
--- a/doc/tar.texi
+++ b/doc/tar.texi
@@ -3086,6 +3086,15 @@ Used when creating an archive. Prevents @command{tar}
from recursing into
directories that are on different file systems from the current
directory.
+@opsummary{one-top-level}
+@item --one-top-level
+
+Tells @command{tar} that only one name from the archive should be placed
directly
+under the extraction directory (the working directory or the one specified with
+@option{-C}). A fatal error is thrown if a second one is found. The top-level
of
+a member is determined after the transformations from
@option{--strip-components}
+and @option{--transform} are applied.
+
@opsummary{overwrite}
@item --overwrite
diff --git a/src/common.h b/src/common.h
index 42fd539..f3f7e4a 100644
--- a/src/common.h
+++ b/src/common.h
@@ -235,6 +235,9 @@ GLOBAL bool numeric_owner_option;
GLOBAL bool one_file_system_option;
+/* Refuse to extract more than one top-level name. */
+GLOBAL bool one_top_level_option;
+
/* Specified value to be put into tar file in place of stat () results, or
just null and -1 if such an override should not take place. */
GLOBAL char const *owner_name_option;
diff --git a/src/extract.c b/src/extract.c
index 9b6b7f9..ea5be67 100644
--- a/src/extract.c
+++ b/src/extract.c
@@ -1578,6 +1578,45 @@ prepare_to_extract (char const *file_name, int typeflag,
tar_extractor_t *fun)
return 1;
}
+/* Throws an error if passed file name is the second top-level name. */
+void
+depth_check (char *file_name)
+{
+ size_t i;
+ size_t start = 0;
+
+ static char *first_top_level;
+ static bool first_top_level_found = false;
+
+ /* Leading slashes have already been removed so it is safe to start at 1. */
+ for (i = 1; i = strlen (file_name); i++)
+{
+ if (ISSLASH (file_name[i]) || file_name[i] == '\0')
+{
+ if ((i == start) || (i - start == 1 file_name[start] == '.'))
+ start = i + 1;
+ else
+ {
+ if (first_top_level_found)
+ {
+ /* Still reject two top-levels if one is a substring of the
other. */
+ if (strncmp (first_top_level, file_name + start, i - start)
+ || strlen (first_top_level) != i - start)
+ FATAL_ERROR ((0, 0, _(Found multiple top-level names,
exiting)));
+ }
+ else
+ {
+ first_top_level_found = true;
+ first_top_level = xmalloc (i - start + 1);
+ strncpy (first_top_level, file_name + start, i - start);
+ first_top_level[i - start] = '\0';
+ }
+ return;
+ }
+ }
+}
+}
+
/* Extract a file from the archive. */
void
extract_archive (void)
@@ -1623,6 +1662,10 @@ extract_archive (void)
return;
}
+ /* Check for names where the depth is too high. */
+ if (one_top_level_option)
+depth_check (current_stat_info.file_name);
+
/* Extract the archive entry according to its type. */
/* KLUDGE */
typeflag = sparse_member_p (current_stat_info) ?
diff --git a/src/tar.c b/src/tar.c
index 4f5017d..8c3014e 100644
--- a/src/tar.c
+++ b/src/tar.c
@@ -319,6 +319,7 @@ enum
OCCURRENCE_OPTION,
OLD_ARCHIVE_OPTION,
ONE_FILE_SYSTEM_OPTION,
+ ONE_TOP_LEVEL_OPTION,
OVERWRITE_DIR_OPTION,
OVERWRITE_OPTION,
OWNER_OPTION,
@@ -489,6 +490,8 @@ static struct argp_option options[] = {
{keep-directory-symlink, KEEP_DIRECTORY_SYMLINK_OPTION, 0, 0,
N_(preserve existing symlinks to directories when extracting),
GRID+1 },
+ {one-top-level, ONE_TOP_LEVEL_OPTION, 0, 0,
+ N_(allow at most one top-level name when
[Bug-tar] [PATCH] Option to treat tarbombs as errors
* src/common.h (one_top_level_option): New global.
* src/extract.c (extract_archive): If one_top_level_option is set, check
for whether the archive tries to create two file trees.
* src/tar.c (ONE_TOP_LEVEL_OPTION): New constant.
(options): New option --one-top-level.
(parse_opt): Handle this option.
(decode_options): Make it conflict with --absolute-names.
* NEWS: Update.
* doc/tar.texi: Document new option.
---
NEWS | 10 ++
doc/tar.texi | 9 +
src/common.h | 3 +++
src/extract.c | 43 +++
src/tar.c | 10 ++
5 files changed, 75 insertions(+)
diff --git a/NEWS b/NEWS
index ffeb4b8..88f8d2c 100644
--- a/NEWS
+++ b/NEWS
@@ -1,6 +1,16 @@
GNU tar NEWS - User visible changes. 2013-10-05
Please send GNU tar bug reports to bug-tar@gnu.org
+
+version 1.27.90 (Git)
+
+* The --one-top-level option.
+
+This new command line option tells tar that the working directory
+(or the one passed to -C) should not be populated with more than one
+name directly under it. A fatal error is thrown if, after name
+transformations, a second member that would be extracted to this location
+is found.
version 1.27 - Sergey Poznyakoff, 2013-10-05
diff --git a/doc/tar.texi b/doc/tar.texi
index 9fde5a0..86d7064 100644
--- a/doc/tar.texi
+++ b/doc/tar.texi
@@ -3086,6 +3086,15 @@ Used when creating an archive. Prevents @command{tar}
from recursing into
directories that are on different file systems from the current
directory.
+@opsummary{one-top-level}
+@item --one-top-level
+
+Tells @command{tar} that only one name from the archive should be placed
directly
+under the extraction directory (the working directory or the one specified with
+@option{-C}). A fatal error is thrown if a second one is found. The top-level
of
+a member is determined after the transformations from
@option{--strip-components}
+and @option{--transform} are applied.
+
@opsummary{overwrite}
@item --overwrite
diff --git a/src/common.h b/src/common.h
index eb801bb..fcf1b7d 100644
--- a/src/common.h
+++ b/src/common.h
@@ -235,6 +235,9 @@ GLOBAL bool numeric_owner_option;
GLOBAL bool one_file_system_option;
+/* Refuse to extract more than one top-level name. */
+GLOBAL bool one_top_level_option;
+
/* Specified value to be put into tar file in place of stat () results, or
just null and -1 if such an override should not take place. */
GLOBAL char const *owner_name_option;
diff --git a/src/extract.c b/src/extract.c
index 9b6b7f9..ea5be67 100644
--- a/src/extract.c
+++ b/src/extract.c
@@ -1578,6 +1578,45 @@ prepare_to_extract (char const *file_name, int typeflag,
tar_extractor_t *fun)
return 1;
}
+/* Throws an error if passed file name is the second top-level name. */
+void
+depth_check (char *file_name)
+{
+ size_t i;
+ size_t start = 0;
+
+ static char *first_top_level;
+ static bool first_top_level_found = false;
+
+ /* Leading slashes have already been removed so it is safe to start at 1. */
+ for (i = 1; i = strlen (file_name); i++)
+{
+ if (ISSLASH (file_name[i]) || file_name[i] == '\0')
+{
+ if ((i == start) || (i - start == 1 file_name[start] == '.'))
+ start = i + 1;
+ else
+ {
+ if (first_top_level_found)
+ {
+ /* Still reject two top-levels if one is a substring of the
other. */
+ if (strncmp (first_top_level, file_name + start, i - start)
+ || strlen (first_top_level) != i - start)
+ FATAL_ERROR ((0, 0, _(Found multiple top-level names,
exiting)));
+ }
+ else
+ {
+ first_top_level_found = true;
+ first_top_level = xmalloc (i - start + 1);
+ strncpy (first_top_level, file_name + start, i - start);
+ first_top_level[i - start] = '\0';
+ }
+ return;
+ }
+ }
+}
+}
+
/* Extract a file from the archive. */
void
extract_archive (void)
@@ -1623,6 +1662,10 @@ extract_archive (void)
return;
}
+ /* Check for names where the depth is too high. */
+ if (one_top_level_option)
+depth_check (current_stat_info.file_name);
+
/* Extract the archive entry according to its type. */
/* KLUDGE */
typeflag = sparse_member_p (current_stat_info) ?
diff --git a/src/tar.c b/src/tar.c
index 4f5017d..8c3014e 100644
--- a/src/tar.c
+++ b/src/tar.c
@@ -319,6 +319,7 @@ enum
OCCURRENCE_OPTION,
OLD_ARCHIVE_OPTION,
ONE_FILE_SYSTEM_OPTION,
+ ONE_TOP_LEVEL_OPTION,
OVERWRITE_DIR_OPTION,
OVERWRITE_OPTION,
OWNER_OPTION,
@@ -489,6 +490,8 @@ static struct argp_option options[] = {
{keep-directory-symlink, KEEP_DIRECTORY_SYMLINK_OPTION, 0, 0,
N_(preserve existing symlinks to directories when extracting),
GRID+1 },
+ {one-top-level, ONE_TOP_LEVEL_OPTION, 0, 0,
+ N_(allow at most one top-level name when
[Bug-tar] [PATCH] Option to treat tarbombs as errors
There has been recent discussion around whether GNU tar should have an
option to deal with archives that do not follow the practice of having
everything in one directory. As an alternative to more involved patches
that have been proposed, this simply adds a new error to help the user
not see as many unpleasant surprises. Various workarounds could then be
used on an individual basis.
Signed-off-by: Connor Behan connor.be...@gmail.com
---
doc/tar.texi | 7 +++
src/common.h | 3 +++
src/extract.c | 23 +++
src/tar.c | 7 +++
4 files changed, 40 insertions(+)
diff --git a/doc/tar.texi b/doc/tar.texi
index 2661174..ef76cfe 100644
--- a/doc/tar.texi
+++ b/doc/tar.texi
@@ -3270,6 +3270,13 @@ the archive creation operations it instructs
@command{tar} to list the
member names stored in the archive, as opposed to the actual file
names. @xref{listing member and file names}.
+@opsummary{single-top-level}
+@item --single-top-level
+
+Tells @command{tar} to extract only one name from the archive into its
+parent directory or the one specified with @option{-C}. A fatal error is
+thrown if a second one is found.
+
@opsummary{skip-old-files}
@item --skip-old-files
diff --git a/src/common.h b/src/common.h
index 21df8c1..0cf7da3 100644
--- a/src/common.h
+++ b/src/common.h
@@ -270,6 +270,9 @@ GLOBAL size_t strip_name_components;
GLOBAL bool show_omitted_dirs_option;
+/* Refuse to extract more than one name that does not contain a slash */
+GLOBAL bool single_top_level_option;
+
GLOBAL bool sparse_option;
GLOBAL unsigned tar_sparse_major;
GLOBAL unsigned tar_sparse_minor;
diff --git a/src/extract.c b/src/extract.c
index 319aaa8..8f77a88 100644
--- a/src/extract.c
+++ b/src/extract.c
@@ -1559,6 +1559,25 @@ prepare_to_extract (char const *file_name, int typeflag,
tar_extractor_t *fun)
return 1;
}
+/* Throws an error if passed file name is the second top-level name. */
+void
+top_level_check (char *file_name)
+{
+ static bool has_top_level = false;
+ int i;
+
+ for (i = 0; i strlen (file_name); i++)
+ if (ISSLASH (file_name[i])) break;
+
+ if (i == strlen (file_name))
+{
+ if (has_top_level)
+FATAL_ERROR ((0, 0, _(More than one top-level name found, exiting)));
+
+ has_top_level = true;
+}
+}
+
/* Extract a file from the archive. */
void
extract_archive (void)
@@ -1604,6 +1623,10 @@ extract_archive (void)
return;
}
+ /* Ensure that we are not extracting more than one top-level file. */
+ if (single_top_level_option)
+ top_level_check (current_stat_info.file_name);
+
/* Extract the archive entry according to its type. */
/* KLUDGE */
typeflag = sparse_member_p (current_stat_info) ?
diff --git a/src/tar.c b/src/tar.c
index c3c2459..1f4c9da 100644
--- a/src/tar.c
+++ b/src/tar.c
@@ -338,6 +338,7 @@ enum
SHOW_DEFAULTS_OPTION,
SHOW_OMITTED_DIRS_OPTION,
SHOW_TRANSFORMED_NAMES_OPTION,
+ SINGLE_TOP_LEVEL_OPTION,
SKIP_OLD_FILES_OPTION,
SPARSE_VERSION_OPTION,
STRIP_COMPONENTS_OPTION,
@@ -484,6 +485,8 @@ static struct argp_option options[] = {
{overwrite-dir, OVERWRITE_DIR_OPTION, 0, 0,
N_(overwrite metadata of existing directories when extracting (default)),
GRID+1 },
+ {single-top-level, SINGLE_TOP_LEVEL_OPTION, 0, 0,
+ N_(allow at most one top-level name when extracting), GRID+1 },
#undef GRID
#define GRID 40
@@ -1548,6 +1551,10 @@ parse_opt (int key, char *arg, struct argp_state *state)
sparse_option = true;
break;
+case SINGLE_TOP_LEVEL_OPTION:
+ single_top_level_option = true;
+ break;
+
case SKIP_OLD_FILES_OPTION:
old_files_option = SKIP_OLD_FILES;
break;
--
1.8.3.4
