While auditing coreutils I noticed a potential buffer overrun in
copy.c on hosts like GNU/Hurd where symbolic links have potentially
unlimited length. Here's a patch. As a nice side effect, this patch
improves the performance of programs on 'ls' on more-ordinary hosts
when they are given long symlinks, as it causes readlink to be invoked
once, rather than twice, when the symlink is long.
2004-05-25 Paul Eggert <[EMAIL PROTECTED]>
Improve the efficiency (and in one case, correctness) of code
that reads symlinks.
* lib/xreadlink.c: Include xreadlink.h first, to catch .h file
dependency problems.
(xreadlink): Accept new arg SIZE, for efficiency.
All decls and uses changed.
* lib/xreadlink.h: Include <stddef.h>, for size_t.
* src/copy.c (copy_internal): Don't use alloca, as it can mess up
royally if the link length is long (e.g., GNU/Hurd). Use
xreadlink instead, it's safer. Don't bother to read the link if
it's the wrong size. Add a FIXME because this area is a bit murky
and undocumented.
Index: lib/canonicalize.c
===================================================================
RCS file: /home/meyering/coreutils/cu/lib/canonicalize.c,v
retrieving revision 1.10
diff -p -u -r1.10 canonicalize.c
--- lib/canonicalize.c 29 Mar 2004 07:29:06 -0000 1.10
+++ lib/canonicalize.c 25 May 2004 22:05:46 -0000
@@ -1,5 +1,5 @@
/* Return the canonical absolute name of a given file.
- Copyright (C) 1996-2001, 2002, 2003 Free Software Foundation, Inc.
+ Copyright (C) 1996-2001, 2002, 2003, 2004 Free Software Foundation, Inc.
This program is free software; you can redistribute it and/or modify
it under the terms of the GNU General Public License as published by
@@ -260,7 +260,7 @@ canonicalize_file_name (const char *name
}
# endif /* MAXSYMLINKS */
- buf = xreadlink (rpath);
+ buf = xreadlink (rpath, st.st_size);
if (!buf)
goto error;
Index: lib/xreadlink.c
===================================================================
RCS file: /home/meyering/coreutils/cu/lib/xreadlink.c,v
retrieving revision 1.12
diff -p -u -r1.12 xreadlink.c
--- lib/xreadlink.c 21 Nov 2003 08:21:23 -0000 1.12
+++ lib/xreadlink.c 25 May 2004 21:57:36 -0000
@@ -1,6 +1,6 @@
/* xreadlink.c -- readlink wrapper to return the link name in malloc'd storage
- Copyright (C) 2001, 2003 Free Software Foundation, Inc.
+ Copyright (C) 2001, 2003, 2004 Free Software Foundation, Inc.
This program is free software; you can redistribute it and/or modify
it under the terms of the GNU General Public License as published by
@@ -23,6 +23,8 @@
# include <config.h>
#endif
+#include "xreadlink.h"
+
#include <stdio.h>
#include <errno.h>
#ifndef errno
@@ -44,20 +46,21 @@ extern int errno;
#endif
#include "xalloc.h"
-#include "xreadlink.h"
/* Call readlink to get the symbolic link value of FILENAME.
+ SIZE is a hint as to how long the link is expected to be;
+ typically it is taken from st_size. It need not be correct.
Return a pointer to that NUL-terminated string in malloc'd storage.
If readlink fails, return NULL (caller may use errno to diagnose).
If malloc fails, or if the link value is longer than SSIZE_MAX :-),
give a diagnostic and exit. */
char *
-xreadlink (char const *filename)
+xreadlink (char const *filename, size_t size)
{
/* The initial buffer size for the link value. A power of 2
detects arithmetic overflow earlier, but is not required. */
- size_t buf_size = 128;
+ size_t buf_size = size + 1;
while (1)
{
@@ -80,7 +83,7 @@ xreadlink (char const *filename)
free (buffer);
buf_size *= 2;
- if (SSIZE_MAX < buf_size || (SIZE_MAX / 2 < SSIZE_MAX && buf_size == 0))
+ if (! (0 < buf_size && buf_size <= SSIZE_MAX))
xalloc_die ();
}
}
Index: lib/xreadlink.h
===================================================================
RCS file: /home/meyering/coreutils/cu/lib/xreadlink.h,v
retrieving revision 1.4
diff -p -u -r1.4 xreadlink.h
--- lib/xreadlink.h 18 Jun 2003 08:03:45 -0000 1.4
+++ lib/xreadlink.h 25 May 2004 21:57:29 -0000
@@ -1,6 +1,6 @@
/* readlink wrapper to return the link name in malloc'd storage
- Copyright (C) 2001, 2003 Free Software Foundation, Inc.
+ Copyright (C) 2001, 2003, 2004 Free Software Foundation, Inc.
This program is free software; you can redistribute it and/or modify
it under the terms of the GNU General Public License as published by
@@ -19,4 +19,5 @@
/* Written by Jim Meyering <[EMAIL PROTECTED]> */
-char *xreadlink (char const *);
+#include <stddef.h>
+char *xreadlink (char const *, size_t);
Index: src/copy.c
===================================================================
RCS file: /home/meyering/coreutils/cu/src/copy.c,v
retrieving revision 1.162
diff -p -u -r1.162 copy.c
--- src/copy.c 27 Apr 2004 15:01:31 -0000 1.162
+++ src/copy.c 25 May 2004 22:04:43 -0000
@@ -1475,7 +1475,7 @@ copy_internal (const char *src_path, con
#ifdef S_ISLNK
if (S_ISLNK (src_type))
{
- char *src_link_val = xreadlink (src_path);
+ char *src_link_val = xreadlink (src_path, src_sb.st_size);
if (src_link_val == NULL)
{
error (0, errno, _("cannot read symbolic link %s"), quote (src_path));
@@ -1487,17 +1487,18 @@ copy_internal (const char *src_path, con
else
{
int saved_errno = errno;
- int same_link = 0;
- if (x->update && !new_dst && S_ISLNK (dst_sb.st_mode))
+ bool same_link = false;
+ if (x->update && !new_dst && S_ISLNK (dst_sb.st_mode)
+ && dst_sb.st_size == strlen (src_link_val))
{
- /* See if the destination is already the desired symlink. */
- size_t src_link_len = strlen (src_link_val);
- char *dest_link_val = alloca (src_link_len + 1);
- int dest_link_len = readlink (dst_path, dest_link_val,
- src_link_len + 1);
- if ((size_t) dest_link_len == src_link_len
- && strncmp (dest_link_val, src_link_val, src_link_len) == 0)
- same_link = 1;
+ /* See if the destination is already the desired symlink.
+ FIXME: This behavior isn't documented, and seems wrong
+ in some cases, e.g., if the destination symlink has the
+ wrong ownership, permissions, or time stamps. */
+ char *dest_link_val = xreadlink (dst_path, dst_sb.st_size);
+ if (strcmp (dest_link_val, src_link_val) == 0)
+ same_link = true;
+ free (dest_link_val);
}
free (src_link_val);
Index: src/ls.c
===================================================================
RCS file: /home/meyering/coreutils/cu/src/ls.c,v
retrieving revision 1.355
diff -p -u -r1.355 ls.c
--- src/ls.c 24 Apr 2004 08:03:15 -0000 1.355
+++ src/ls.c 25 May 2004 21:45:58 -0000
@@ -2641,7 +2641,7 @@ gobble_file (const char *name, enum file
static void
get_link_name (const char *filename, struct fileinfo *f)
{
- f->linkname = xreadlink (filename);
+ f->linkname = xreadlink (filename, f->stat.st_size);
if (f->linkname == NULL)
{
error (0, errno, _("cannot read symbolic link %s"),
Index: src/readlink.c
===================================================================
RCS file: /home/meyering/coreutils/cu/src/readlink.c,v
retrieving revision 1.9
diff -p -u -r1.9 readlink.c
--- src/readlink.c 18 Oct 2003 10:05:47 -0000 1.9
+++ src/readlink.c 25 May 2004 21:58:00 -0000
@@ -1,5 +1,5 @@
/* readlink -- display value of a symbolic link.
- Copyright (C) 2002, 2003 Free Software Foundation, Inc.
+ Copyright (C) 2002, 2003, 2004 Free Software Foundation, Inc.
This program is free software; you can redistribute it and/or modify
it under the terms of the GNU General Public License as published by
@@ -137,7 +137,9 @@ main (int argc, char *const argv[])
usage (EXIT_FAILURE);
}
- value = (canonicalize ? canonicalize_file_name : xreadlink) (fname);
+ value = (canonicalize
+ ? canonicalize_file_name (fname)
+ : xreadlink (fname, 1024));
if (value)
{
printf ("%s%s", value, (no_newline ? "" : "\n"));
Index: src/stat.c
===================================================================
RCS file: /home/meyering/coreutils/cu/src/stat.c,v
retrieving revision 1.69
diff -p -u -r1.69 stat.c
--- src/stat.c 9 Apr 2004 12:03:02 -0000 1.69
+++ src/stat.c 25 May 2004 21:48:09 -0000
@@ -433,7 +433,7 @@ print_stat (char *pformat, char m, char
strcat (pformat, "s");
if (S_ISLNK (statbuf->st_mode))
{
- char *linkname = xreadlink (filename);
+ char *linkname = xreadlink (filename, statbuf->st_size);
if (linkname == NULL)
{
error (0, errno, _("cannot read symbolic link %s"),
_______________________________________________
Bug-coreutils mailing list
[EMAIL PROTECTED]
http://mail.gnu.org/mailman/listinfo/bug-coreutils
