On Mon, 19 Feb 2007 12:52:12 +0100, Raúl Sánchez Siles wrote:
> To reproduce the error do this:
> 
> 1- echo "filename" > filelist
> 2- echo "" >>filelist
> 3- tar xvfz tarfile.tgz -T filelist
> 
> Note: its not necessary that "filename" exists.
>
> When you use -T, the contents of the file is dumped into the argv
> vector. Each line should contain a file name, optionally with its path.
> 
> You will have a segment violation because on line 421 of lib/getopt.c
> you have:
> 
> if (d->optind != argc && !strcmp (argv[d->optind], "--"))

Hi Raúl,

Thank you very much for the bug report.

As it turns out, the bug may or may not a segmentation fault. To be
precise, the code is passing a NULL pointer to strcmp, which is
undefined behavior according the the relevant specification. So
conforming implementations can do anything here, (including a
segmentation fault as well as silently treating NULL as an empty
string). My system, for example, seems to behave with the
silent-treat-NULL-like-empty-string behavior.

Regardless, this undefined behavior is something the program should
avoid doing.

> There are two possible options as I see it.
> 
> 1- Changing that line to: 
> if (d->optind != argc && argv[d->optind] && !strcmp (argv[d->optind], "--"))
> 
> 2- Fix the parsing of -T so an empty line won't be included in the dump
> to argv vector.

I've attached a patch for #1. Doing #2 looked like a bit more work to
me, and I'm lazy that way, (but Sergey might want to do something about
it).

> I tried to report this upstream but bug reporting is limited to memebers
> of the project.

I don't know if anything has changed with respect to bug reporting for
upstream tar in the past couple of years. But I've started reporting
bugs recently, and have had no problem doing so without any membership.
I simply send bug reports to bug-...@gnu.org, (as I am doing with this
email), and I've found upstream to be particularly responsive to
high-quality bug reports, (thanks, Sergey!).

Happy hacking,

-Carl

From 0328eb91a46840bf5020a7f627aa5664a2948621 Mon Sep 17 00:00:00 2001
From: =?utf-8?q?Ra=C3=BAl=20S=C3=A1nchez=20Siles?= <r...@barracuda.es>
Date: Tue, 4 Aug 2009 14:17:12 -0700
Subject: [PATCH] Avoid undefined behavior of passing NULL to strcmp

Having a blank line in the file passed to the -T option will cause
an entry in the argv array to be NULL. To avoid invoking undefined
behavior, we take care not to pass these NULL values to strcmp.
---
 debian/changelog |    4 +++-
 lib/getopt.c     |    2 +-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/debian/changelog b/debian/changelog
index 1e26c81..3238ff3 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -7,8 +7,10 @@ tar (1.22-1.2) UNRELEASED; urgency=low
     Thanks to Ted T'so for the idea and Sergey Poznyakoff for
     cleaning up my original implementation.
   * Respect DEB_BUILD_OPTIONS=nocheck to conform with Policy 3.8.2
+  * Avoid undefined behavior of passing NULL to strcmp, closes: #411485
+    Thanks to Raúl Sánchez Siles for proposing this patch.
 
- -- Carl Worth <cwo...@cworth.org>  Tue, 04 Aug 2009 12:07:06 -0700
+ -- Carl Worth <cwo...@cworth.org>  Tue, 04 Aug 2009 14:16:09 -0700
 
 tar (1.22-1.1) unstable; urgency=low
 
diff --git a/lib/getopt.c b/lib/getopt.c
index f1e6d1f..f1c0e1f 100644
--- a/lib/getopt.c
+++ b/lib/getopt.c
@@ -413,7 +413,7 @@ _getopt_internal_r (int argc, char **argv, const char *optstring,
 	 then exchange with previous non-options as if it were an option,
 	 then skip everything else like a non-option.  */
 
-      if (d->optind != argc && !strcmp (argv[d->optind], "--"))
+      if (d->optind != argc && argv[d->optind] && !strcmp (argv[d->optind], "--"))
 	{
 	  d->optind++;
 
-- 
1.6.3.3

Attachment: signature.asc
Description: This is a digitally signed message part

Reply via email to