Package: coreutils;
Reported by: Mike Frysinger <vapier <at> gentoo.org>
Date: Tue, 10 Jan 2012 20:17:02 UTC
Severity: normal
Done: Pádraig Brady <P <at> draigBrady.com>
Bug is archived. No further changes may be made.
Message #55 received at 10472 <at> debbugs.gnu.org (full text, mbox):
From: Eric Blake <eblake <at> redhat.com> Cc: bug-gnulib <at> gnu.org, 10472 <at> debbugs.gnu.org Subject: Re: bug#10472: [PATCH] canonicalize: fix // handling Date: Sat, 04 Feb 2012 11:38:44 -0700
[Message part 1 (text/plain, inline)]
On 02/04/2012 10:59 AM, Eric Blake wrote:
> On 02/04/2012 09:56 AM, Eric Blake wrote:
>> On Cygwin, and other platforms where // is detected as distinct
>> from / at configure time, the canonicalize routines were incorrectly
>> treating all instances of multiple leading slashes as //.
>> See also coreutils bug http://debbugs.gnu.org/10472
>>
>> * lib/canonicalize.c (canonicalize_filename_mode): Don't convert
>> /// to //, since only // is special.
>>
>
> which meant this was reading uninitialized memory, and depending on what
> was in the heap, might canonicalize "///" to "/" or "//". I'm pushing
> this additional fix to both files:
>
> diff --git i/lib/canonicalize-lgpl.c w/lib/canonicalize-lgpl.c
> index a61bef9..08e76fe 100644
> --- i/lib/canonicalize-lgpl.c
> +++ w/lib/canonicalize-lgpl.c
> @@ -158,6 +158,7 @@ __realpath (const char *name, char *resolved)
> dest = rpath + 1;
> if (DOUBLE_SLASH_IS_DISTINCT_ROOT && name[1] == '/' && name[2] !=
> '/')
> *dest++ = '/';
> + *dest = '\0';
> }
Still not right. If you have a symlink at //some/path whose contents is
/, then that would canonicalize to '//' without triggering any valgrind
complaints, because I missed the code that resets rpath on encountering
absolute symlink contents. Meanwhile, pre-assigning *dest is a
pessimization on platforms where // and / are identical. I'm pushing
this instead.
From d1f3998942236194f1894c45804ec947d07ed134 Mon Sep 17 00:00:00 2001
From: Eric Blake <eblake <at> redhat.com>
Date: Sat, 4 Feb 2012 11:11:40 -0700
Subject: [PATCH] canonicalize: avoid uninitialized memory use
When DOUBLE_SLASH_IS_DISTINCT_ROOT is non-zero, then we were
reading the contents of rpath[1] even when we had never written
anything there, which meant that "///" would usually canonicalize
to "/" but sometimes to "//" if a '/' was leftover in the heap.
This condition could also occur via 'ln -s / //some/path' and
canonicalizing //some/path, where we rewind rpath but do not
clear out the previous round. Platforms where "//" and "/" are
equivalent do not suffer from this read-beyond-written bounds.
* lib/canonicalize-lgpl.c (__realpath): Avoid possibility of
random '/' left in dest.
* lib/canonicalize.c (canonicalize_filename_mode): Likewise.
Signed-off-by: Eric Blake <eblake <at> redhat.com>
---
ChangeLog | 7 +++++++
lib/canonicalize-lgpl.c | 17 ++++++++++++-----
lib/canonicalize.c | 17 ++++++++++++-----
3 files changed, 31 insertions(+), 10 deletions(-)
diff --git a/ChangeLog b/ChangeLog
index 8f08543..aeea7c8 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,10 @@
+2012-02-04 Eric Blake <eblake <at> redhat.com>
+
+ canonicalize: avoid uninitialized memory use
+ * lib/canonicalize-lgpl.c (__realpath): Avoid possibility of
+ random '/' left in dest.
+ * lib/canonicalize.c (canonicalize_filename_mode): Likewise.
+
2012-02-04 Bruno Haible <bruno <at> clisp.org>
spawn-pipe tests: Fix a NULL program name in a diagnostic.
diff --git a/lib/canonicalize-lgpl.c b/lib/canonicalize-lgpl.c
index a61bef9..7aa2d92 100644
--- a/lib/canonicalize-lgpl.c
+++ b/lib/canonicalize-lgpl.c
@@ -156,8 +156,12 @@ __realpath (const char *name, char *resolved)
{
rpath[0] = '/';
dest = rpath + 1;
- if (DOUBLE_SLASH_IS_DISTINCT_ROOT && name[1] == '/' && name[2] !=
'/')
- *dest++ = '/';
+ if (DOUBLE_SLASH_IS_DISTINCT_ROOT)
+ {
+ if (name[1] == '/' && name[2] != '/')
+ *dest++ = '/';
+ *dest = '\0';
+ }
}
for (start = end = name; *start; start = end)
@@ -298,9 +302,12 @@ __realpath (const char *name, char *resolved)
if (buf[0] == '/')
{
dest = rpath + 1; /* It's an absolute symlink */
- if (DOUBLE_SLASH_IS_DISTINCT_ROOT
- && buf[1] == '/' && buf[2] != '/')
- *dest++ = '/';
+ if (DOUBLE_SLASH_IS_DISTINCT_ROOT)
+ {
+ if (buf[1] == '/' && buf[2] != '/')
+ *dest++ = '/';
+ *dest = '\0';
+ }
}
else
{
diff --git a/lib/canonicalize.c b/lib/canonicalize.c
index ed094b7..583c1a4 100644
--- a/lib/canonicalize.c
+++ b/lib/canonicalize.c
@@ -145,8 +145,12 @@ canonicalize_filename_mode (const char *name,
canonicalize_mode_t can_mode)
rname_limit = rname + PATH_MAX;
rname[0] = '/';
dest = rname + 1;
- if (DOUBLE_SLASH_IS_DISTINCT_ROOT && name[1] == '/' && name[2] !=
'/')
- *dest++ = '/';
+ if (DOUBLE_SLASH_IS_DISTINCT_ROOT)
+ {
+ if (name[1] == '/' && name[2] != '/')
+ *dest++ = '/';
+ *dest = '\0';
+ }
}
for (start = name; *start; start = end)
@@ -267,9 +271,12 @@ canonicalize_filename_mode (const char *name,
canonicalize_mode_t can_mode)
if (buf[0] == '/')
{
dest = rname + 1; /* It's an absolute symlink */
- if (DOUBLE_SLASH_IS_DISTINCT_ROOT
- && buf[1] == '/' && buf[2] != '/')
- *dest++ = '/';
+ if (DOUBLE_SLASH_IS_DISTINCT_ROOT)
+ {
+ if (buf[1] == '/' && buf[2] != '/')
+ *dest++ = '/';
+ *dest = '\0';
+ }
}
else
{
--
1.7.7.6
--
Eric Blake eblake <at> redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[signature.asc (application/pgp-signature, attachment)]
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.