vdr-1.4.1: bug report and fix: ReadLink with relative targets

Message ID 448E3A5B.8070707@errror.org
State New
Headers

Commit Message

Patrick Cernko June 13, 2006, 4:08 a.m. UTC
  Hi again,

Patrick Cernko wrote:
> Hi Klaus, hi list,
> 
> today I discovered that the ReadLink function (used in e.g. cSafeFile)
> does not handle relative links correctly. I used a symlinked
> channels.conf like that:
> 
> errror@ds9:/var/lib/vdr> ll channels.conf
> lrwxrwxrwx  1 errror vdr 20 2006-06-12 00:03 channels.conf ->
> channels.conf.normal
> 
> but vdr tried to write to channels.conf.$$$ in its current working
> directory ("/") which failed. :-(
> 
> The bug is caused by the ReadLink function which always took the
> unchanged value of the found link as target (without prepending the
> links path for relative links).
> 
> As a fix, I added a check for relative links (== not starting with a
> '/') and prepend the directory of the symlink. For that purpose I use
> some small parts from the coreutils "dirname" program as I learned for a
> long time that doing something so easy as computing the directory part
> of a path can lead to several errors if not done right. :-)
> 
> Appended you find the patch, which makes vdr handle relative symlinks
> correctly.
> 
> @Klaus: Feel free to review/reduce the code but pay attention to the
> special cases! ;-)
> 

I'm sorry but I made a small mistake in my first attempt resulting in a
wrong control-flow (damn if-else without parentensis) :-(

Here is an updated patch.

So long,
  

Patch

--- vdr-1.4.1.orig/tools.c
+++ vdr-1.4.1/tools.c
@@ -478,6 +478,99 @@ 
   return -1;
 }
 
+// BEGIN: taken from coreutils-5.2.1: dir_name() & base_name() functions and helpers
+#include <stddef.h>
+#include <stdlib.h>
+#include <string.h>
+
+#if FILESYSTEM_ACCEPTS_DRIVE_LETTER_PREFIX
+# define FILESYSTEM_PREFIX_LEN(Filename) \
+  ((Filename)[0] && (Filename)[1] == ':' ? 2 : 0)
+#else
+# define FILESYSTEM_PREFIX_LEN(Filename) 0
+#endif
+
+#if FILESYSTEM_BACKSLASH_IS_FILE_NAME_SEPARATOR
+# define ISSLASH(C) ((C) == '/' || (C) == '\\')
+#else
+# define ISSLASH(C) ((C) == '/')
+#endif
+
+/* In general, we can't use the builtin `basename' function if available,
+   since it has different meanings in different environments.
+   In some environments the builtin `basename' modifies its argument.
+
+   Return the address of the last file name component of NAME.  If
+   NAME has no file name components because it is all slashes, return
+   NAME if it is empty, the address of its last slash otherwise.  */
+
+char *
+base_name (char const *name)
+{
+  char const *base = name + FILESYSTEM_PREFIX_LEN (name);
+  char const *p;
+
+  for (p = base; *p; p++)
+    {
+      if (ISSLASH (*p))
+        {
+          /* Treat multiple adjacent slashes like a single slash.  */
+          do p++;
+          while (ISSLASH (*p));
+
+          /* If the file name ends in slash, use the trailing slash as
+             the basename if no non-slashes have been found.  */
+          if (! *p)
+            {
+              if (ISSLASH (*base))
+                base = p - 1;
+              break;
+            }
+
+          /* *P is a non-slash preceded by a slash.  */
+          base = p;
+        }
+    }
+
+  return (char *) base;
+}
+
+/* Return the length of `dirname (PATH)', or zero if PATH is
+   in the working directory.  Works properly even if
+   there are trailing slashes (by effectively ignoring them).  */
+size_t
+dir_len (char const *path)
+{
+  size_t prefix_length = FILESYSTEM_PREFIX_LEN (path);
+  size_t length;
+
+  /* Strip the basename and any redundant slashes before it.  */
+  for (length = base_name (path) - path;  prefix_length < length;  length--)
+    if (! ISSLASH (path[length - 1]))
+      return length;
+
+  /* But don't strip the only slash from "/".  */
+  return prefix_length + ISSLASH (path[prefix_length]);
+}
+
+/* Return the leading directories part of PATH,
+   allocated with xmalloc.
+   Works properly even if there are trailing slashes
+   (by effectively ignoring them).  */
+char *
+dir_name (char const *path)
+{
+  size_t length = dir_len (path);
+  int append_dot = (length == FILESYSTEM_PREFIX_LEN (path));
+  char *newpath = (char*) malloc (length + append_dot + 1);
+  memcpy (newpath, path, length);
+  if (append_dot)
+    newpath[length++] = '.';
+  newpath[length] = 0;
+  return newpath;
+}
+// END: taken from coreutils-5.2.1
+
 char *ReadLink(const char *FileName)
 {
   char RealName[PATH_MAX];
@@ -489,12 +582,26 @@ 
      else // some other error occurred
         LOG_ERROR_STR(FileName);
      }
-  else if (n < int(sizeof(RealName))) { // got it!
-     RealName[n] = 0;
-     TargetName = RealName;
-     }
-  else
-     esyslog("ERROR: symlink's target name too long: %s", FileName);
+  else {
+    if (RealName[0] != '/') { // relative symlink, we must prepend the path of FileName
+      char* dirname = dir_name(FileName);
+      size_t dirnamelen = strlen(dirname);
+      char linkval[n+1];
+      memcpy(linkval, RealName, n);
+      memcpy(RealName, dirname, dirnamelen);   // first prepend the path of FileName
+      RealName[dirnamelen] = '/';
+      memcpy(RealName+dirnamelen+1, linkval, n); // append the gotten n bytes from the link value
+      n += 1 + dirnamelen;
+      RealName[n] = 0;
+      free(dirname);
+    }
+    if (n < int(sizeof(RealName))) { // got it!
+      RealName[n] = 0;
+      TargetName = RealName;
+    }
+    else
+      esyslog("ERROR: symlink's target name too long: %s", FileName);
+  }
   return TargetName ? strdup(TargetName) : NULL;
 }