Re: [Bug-tar] possible fixes for CVE-2016-6321
Thanks for the heads-up. Yes, it appears the 2003 change was not sufficiently
paranoid about ".." in member names. Luckily, the tar manual still documents the
pre-2003 behavior, so we can restore that behavior as a simple bug fix. I
installed the attached patch into Savannah as one way to do that. This patch
causes 'tar' to issue two diagnostics when given a member name containing "..",
and I suppose tar should be cleaned up at some point to issue just one
diagnostic. The main thing, though, is that the patch is simple and fixes the
security gotcha in question.
I don't view this as a serious bug, as the tar manual has long said that you
should extract untrusted tarballs only into empty directories, and doing that
forestalls the attack even without this patch. (There are other reasons for this
longstanding recommendation.)
From 99ceaad4d0efd8669b373e1f542f7205f2548456 Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@Penguin.CS.UCLA.EDU>
Date: Sat, 29 Oct 2016 21:04:40 -0700
Subject: [PATCH] When extracting, skip ".." members
* NEWS: Document this.
* src/extract.c (extract_archive): Skip members whose names
contain "..".
---
NEWS | 8 +++++++-
src/extract.c | 8 ++++++++
2 files changed, 15 insertions(+), 1 deletion(-)
diff --git a/NEWS b/NEWS
index caa77bc..07daaa7 100644
--- a/NEWS
+++ b/NEWS
@@ -1,9 +1,15 @@
-GNU tar NEWS - User visible changes. 2016-05-27
+GNU tar NEWS - User visible changes. 2016-10-29
Please send GNU tar bug reports to <bug-tar@gnu.org>
version 1.29.90 (Git)
+* Member names containing '..' components are now skipped when extracting.
+
+This fixes tar's behavior to match its documentation, and is a bit
+safer when extracting untrusted archives over old files (an unsafe
+practice that the tar manual has long recommended against).
+
* Report erroneous use of positional options.
During archive creation or update, tar keeps track of positional
diff --git a/src/extract.c b/src/extract.c
index f982433..7904148 100644
--- a/src/extract.c
+++ b/src/extract.c
@@ -1629,12 +1629,20 @@ extract_archive (void)
{
char typeflag;
tar_extractor_t fun;
+ bool skip_dotdot_name;
fatal_exit_hook = extract_finish;
set_next_block_after (current_header);
+ skip_dotdot_name = (!absolute_names_option
+ && contains_dot_dot (current_stat_info.orig_file_name));
+ if (skip_dotdot_name)
+ ERROR ((0, 0, _("%s: Member name contains '..'"),
+ quotearg_colon (current_stat_info.orig_file_name)));
+
if (!current_stat_info.file_name[0]
+ || skip_dotdot_name
|| (interactive_option
&& !confirm ("extract", current_stat_info.file_name)))
{
--
2.7.4
Reply to: