Subject: bug#17010: closed (Re: bug#17010: Bug#739752: coreutils: ln segfaults when run with --relative and an empty target)

From: [email-masked] (GNU bug Tracking System)
Date: Fri, 14 Mar 2014 16:28:03 +0000

Your bug report

#17010: Bug#739752: coreutils: ln segfaults when run with --relative and an empty target

which was filed against the coreutils package, has been closed.

The explanation is attached below, along with your original report.
If you require more details, please reply to [email-masked].

-- 
17010: http://debbugs.gnu.org/cgi/bugreport.cgi?bug=17010
GNU Bug Tracking System
Contact [email-masked] with problems

Subject: Re: bug#17010: Bug#739752: coreutils: ln segfaults when run with --relative and an empty target
From: Jim Meyering <[email-masked]>
To: Pádraig Brady <[email-masked]>
Cc: Erik Bernstein <[email-masked]>, 17010 <[email-masked]>, 739752 <[email-masked]>
Date: Fri, 14 Mar 2014 09:27:24 -0700
List-Id: <debbugs-submit.debbugs.gnu.org>

On Fri, Mar 14, 2014 at 4:49 AM, Pádraig Brady <[email-masked]> wrote:
...
> Hence since c_f_m() can validly fail even with CAN_MISSING,
> I agree your patch is correct.
>
> Please push.

Done.

Subject: Re: Bug#739752: coreutils: ln segfaults when run with --relative and an empty target
From: Jim Meyering <[email-masked]>
To: Erik Bernstein <[email-masked]>, [email-masked], [email-masked]
Cc: Debian Bug Tracking System <[email-masked]>
Date: Thu, 13 Mar 2014 18:42:30 -0700
List-Id: <debbugs-submit.debbugs.gnu.org>

On Sat, Feb 22, 2014 at 1:57 AM, Erik Bernstein <[email-masked]> wrote:
> Package: coreutils
> Version: 8.21-1
> Severity: normal
>
> Hi,
>
> when ln is run with --relative --symbolic and and empty string as the
> target, it ungracefully dies with a segmentation fault. The memory
> violation appears to happen in src/relpath.c:38 when the two input paths
> are checked for leading slashes:
>
>   if ((path1[1] == '/') != (path2[1] == '/'))
>
> How to reproduce:
> [1] Open a terminal
> [2] run: ln -sr '' foobar
>
> Result: segmentation fault  ln -sr '' foobar
> Expected result: Some kind of error message
...

Thank you for the bug report!
That also affected the very latest code in git.
Here is a patch:
From a6d2db8b6dfe15344aba4aefe9545eb3a4876d45 Mon Sep 17 00:00:00 2001
From: Jim Meyering <[email-masked]>
Date: Thu, 13 Mar 2014 17:05:04 -0700
Subject: [PATCH] ln: with -sr, don't segfault for a TARGET of ''

Prior to this change, "ln -sr '' F" would segfault, attempting
to read path2[1] in relpath.c's path_common_prefix function.
This problem arises whenever canonicalize_filename_mode returns
NULL.
* src/ln.c (convert_abs_rel): Call relpath only when
both canonicalize_filename_mode calls return non-NULL.
* tests/ln/relative.sh: Add a test to trigger this failure.
* THANKS.in: List reporter's name/address.
* NEWS (Bug fixes): Mention it.
Reported by Erik Bernstein in [email-masked].
---
 NEWS                 |  2 ++
 THANKS.in            |  1 +
 src/ln.c             | 16 ++++++++++------
 tests/ln/relative.sh |  5 +++++
 4 files changed, 18 insertions(+), 6 deletions(-)

diff --git a/NEWS b/NEWS
index 62966b2..b3ad65c 100644
--- a/NEWS
+++ b/NEWS
@@ -25,6 +25,8 @@ GNU coreutils NEWS                                    -*- outline -*-
   it would display an error, requiring --no-dereference to avoid the issue.
   [bug introduced in coreutils-5.3.0]

+  ln -sr '' F no longer segfaults: now it fails with the expected diagnostic
+
   shuf --repeat no longer dumps core if the input is empty.
   [bug introduced with the --repeat feature in coreutils-8.22]

diff --git a/THANKS.in b/THANKS.in
index 561d18c..2670265 100644
--- a/THANKS.in
+++ b/THANKS.in
@@ -193,6 +193,7 @@ Eric G. Miller                      [email-masked]
 Eric Pemente                        [email-masked]
 Eric S. Raymond                     [email-masked]
 Erik Bennett                        [email-masked]
+Erik Bernstein                      [email-masked]
 Erik Corry                          [email-masked]
 Felix Lee                           [email-masked]
 Felix Rauch Valenti                 [email-masked]
diff --git a/src/ln.c b/src/ln.c
index aab9cf2..6726699 100644
--- a/src/ln.c
+++ b/src/ln.c
@@ -149,13 +149,17 @@ convert_abs_rel (const char *from, const char *target)
   char *realdest = canonicalize_filename_mode (targetdir, CAN_MISSING);
   char *realfrom = canonicalize_filename_mode (from, CAN_MISSING);

-  /* Write to a PATH_MAX buffer.  */
-  char *relative_from = xmalloc (PATH_MAX);
-
-  if (!relpath (realfrom, realdest, relative_from, PATH_MAX))
+  char *relative_from = NULL;
+  if (realdest && realfrom)
     {
-      free (relative_from);
-      relative_from = NULL;
+      /* Write to a PATH_MAX buffer.  */
+      relative_from = xmalloc (PATH_MAX);
+
+      if (!relpath (realfrom, realdest, relative_from, PATH_MAX))
+        {
+          free (relative_from);
+          relative_from = NULL;
+        }
     }

   free (targetdir);
diff --git a/tests/ln/relative.sh b/tests/ln/relative.sh
index 7636695..e8c25dc 100755
--- a/tests/ln/relative.sh
+++ b/tests/ln/relative.sh
@@ -45,4 +45,9 @@ mkdir web
 ln -sr latest web/latest
 test $(readlink web/latest) = '../release2' || fail=1

+# Expect this to fail with exit status 1.
+# Prior to coreutils-8.23, it would segfault.
+ln -sr '' F
+test $? = 1 || fail=1
+
 Exit $fail
-- 
1.9.0.167.g384364b