Re: Bug#842339: [Bug-tar] possible fixes for CVE-2016-6321

[Salvatore Bonaccorso <carnil@debian.org>]

Control: tags -1 + patch

(dropping the bug-tar list, since this reply only relevant within
Debian).

Hi Paul,

On Sat, Oct 29, 2016 at 09:19:09PM -0700, Paul Eggert wrote:
> 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.)

Thanks for the patch!

For reference, attached would be the debdiff for a possible unstable
and jessie-security upload.

Regards,
Salvatore

diff -Nru tar-1.27.1/debian/changelog tar-1.27.1/debian/changelog
--- tar-1.27.1/debian/changelog	2014-03-22 22:55:23.000000000 +0100
+++ tar-1.27.1/debian/changelog	2016-10-30 07:48:55.000000000 +0100
@@ -1,3 +1,12 @@
+tar (1.27.1-2+deb8u1) jessie-security; urgency=high
+
+  * Non-maintainer upload by the Security Team.
+  * CVE-2016-6321: Bypassing the extract path name.
+    When extracting, member names containing '..' components are skipped.
+    (Closes: #842339)
+
+ -- Salvatore Bonaccorso <carnil@debian.org>  Sun, 30 Oct 2016 07:48:55 +0100
+
 tar (1.27.1-2) unstable; urgency=low
 
   * patch from David Gilman adds watch file with signature verification, 
diff -Nru tar-1.27.1/debian/patches/When-extracting-skip-.-members.patch tar-1.27.1/debian/patches/When-extracting-skip-.-members.patch
--- tar-1.27.1/debian/patches/When-extracting-skip-.-members.patch	1970-01-01 01:00:00.000000000 +0100
+++ tar-1.27.1/debian/patches/When-extracting-skip-.-members.patch	2016-10-30 07:48:55.000000000 +0100
@@ -0,0 +1,33 @@
+Description: When extracting, skip ".." members (CVE-2016-6321)
+Origin: upstream,  http://git.savannah.gnu.org/cgit/tar.git/commit/?id=7340f67b9860ea0531c1450e5aa261c50f67165d
+Bug-Debian: https://bugs.debian.org/842339
+Forwarded: not-needed.
+Author: Paul Eggert <eggert@Penguin.CS.UCLA.EDU>
+Last-Update: 2016-10-30
+---
+ src/extract.c | 8 ++++++++
+ 2 files changed, 15 insertions(+), 1 deletion(-)
+
+--- a/src/extract.c
++++ b/src/extract.c
+@@ -1584,12 +1584,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)))
+     {
diff -Nru tar-1.27.1/debian/patches/series tar-1.27.1/debian/patches/series
--- tar-1.27.1/debian/patches/series	2014-03-22 22:55:23.000000000 +0100
+++ tar-1.27.1/debian/patches/series	2016-10-30 07:48:55.000000000 +0100
@@ -1,2 +1,3 @@
 pristine-tar.diff
 listed03-linux-only
+When-extracting-skip-.-members.patch

diff -Nru tar-1.29b/debian/changelog tar-1.29b/debian/changelog
--- tar-1.29b/debian/changelog	2016-07-22 22:07:14.000000000 +0200
+++ tar-1.29b/debian/changelog	2016-10-30 07:35:31.000000000 +0100
@@ -1,3 +1,12 @@
+tar (1.29b-1.1) unstable; urgency=medium
+
+  * Non-maintainer upload.
+  * CVE-2016-6321: Bypassing the extract path name.
+    When extracting, member names containing '..' components are skipped.
+    (Closes: #842339)
+
+ -- Salvatore Bonaccorso <carnil@debian.org>  Sun, 30 Oct 2016 07:35:31 +0100
+
 tar (1.29b-1) unstable; urgency=medium
 
   * re-constitute the 1.29 orig.tar with man pages as version 1.29b
diff -Nru tar-1.29b/debian/patches/When-extracting-skip-.-members.patch tar-1.29b/debian/patches/When-extracting-skip-.-members.patch
--- tar-1.29b/debian/patches/When-extracting-skip-.-members.patch	1970-01-01 01:00:00.000000000 +0100
+++ tar-1.29b/debian/patches/When-extracting-skip-.-members.patch	2016-10-30 07:35:31.000000000 +0100
@@ -0,0 +1,33 @@
+Description: When extracting, skip ".." members (CVE-2016-6321)
+Origin: upstream,  http://git.savannah.gnu.org/cgit/tar.git/commit/?id=7340f67b9860ea0531c1450e5aa261c50f67165d
+Bug-Debian: https://bugs.debian.org/842339
+Forwarded: not-needed.
+Author: Paul Eggert <eggert@Penguin.CS.UCLA.EDU>
+Last-Update: 2016-10-30
+---
+ src/extract.c | 8 ++++++++
+ 2 files changed, 15 insertions(+), 1 deletion(-)
+
+--- 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)))
+     {
diff -Nru tar-1.29b/debian/patches/series tar-1.29b/debian/patches/series
--- tar-1.29b/debian/patches/series	2016-07-22 22:07:14.000000000 +0200
+++ tar-1.29b/debian/patches/series	2016-10-30 07:35:31.000000000 +0100
@@ -1,3 +1,4 @@
 pristine-tar.diff
 listed03-linux-only
 rmt.8-header-wrong
+When-extracting-skip-.-members.patch