Reimplement reverted commit 7fa35c to fix readlink() issues

Original implementation:

Commit: 7fa35cA
dtfile: coverity CIDs 88363,88405,89140,89612; insecure readlink

That commit caused dtfile to be unable to resolve symbolic links and
was later reverted.  This commit reimplements the fixes correctly, and
should hopefully still resolve the coverity issues as well.

diff --git a/cde/programs/dtfile/Directory.c b/cde/programs/dtfile/Directory.c
index 578e92660a4c163f71b21cbb86240c483e2135c8..618a4b9cc0079d034630d50d25887f14ee2a5fdb 100644 (file)
--- a/cde/programs/dtfile/Directory.c
+++ b/cde/programs/dtfile/Directory.c
@@ -823,9 +823,9 @@ ReadFileData(
    stat_result = lstat (link_file_name, &stat_buf);
    if (stat_result == 0 && (stat_buf.st_mode & S_IFMT) == S_IFLNK)
    {
-     while ((link_len = readlink(link_file_name, link_path, MAX_PATH)) > 0)
+     while ((link_len = readlink(link_file_name, link_path, MAX_PATH - 1)) > 0)
      {
-       link_path[link_len] = '\0';
+       link_path[link_len] = 0;
        link_list = (char **)XtRealloc((char *)link_list, sizeof(char *) *
                                       (link_count + 2));
 
@@ -1069,9 +1069,9 @@ ReadFileData2(
    stat_result = lstat (link_file_name, &stat_buf);
    if ((stat_buf.st_mode & S_IFMT) == S_IFLNK)
    {
-     while ((link_len = readlink(link_file_name, link_path, MAX_PATH)) > 0)
+     while ((link_len = readlink(link_file_name, link_path, MAX_PATH - 1)) > 0)
      {
-       link_path[link_len] = NILL;
+       link_path[link_len] = 0;
        link_list = (char **)XtRealloc((char *)link_list, sizeof(char *) *
                                       (link_count + 2));
 

diff --git a/cde/programs/dtfile/SharedProcs.c b/cde/programs/dtfile/SharedProcs.c
index 48a21aff5a8f8ace222e3766d4e2bf82da0e05c4..c823b432032095d652e056ce32228172df460c67 100644 (file)
--- a/cde/programs/dtfile/SharedProcs.c
+++ b/cde/programs/dtfile/SharedProcs.c
@@ -234,9 +234,9 @@ _DtFollowLink (
 
    strcpy(file, path);
 
-   while ((link_len = readlink(file, link_path, MAXPATHLEN)) > 0)
+   while ((link_len = readlink(file, link_path, MAXPATHLEN - 1)) > 0)
    {
-      link_path[link_len] = '\0';
+      link_path[link_len] = 0;
 
       /* Force the link to be an absolute path, if necessary */
       if (link_path[0] != '/')

diff --git a/cde/programs/dtfile/dtcopy/fsrtns.c b/cde/programs/dtfile/dtcopy/fsrtns.c
index 5532aca2a2dc028713f633b6ac515ac708ef6aa2..fb29f00931e4a65039b90af72b668b9ea2b9e2e6 100644 (file)
--- a/cde/programs/dtfile/dtcopy/fsrtns.c
+++ b/cde/programs/dtfile/dtcopy/fsrtns.c
@@ -179,15 +179,18 @@ CopyLink(char *sourceP, char *targetP, int repl, struct stat *statP)
 /* copy a symbolic link */
 {
   int l, rc;
-  char buf[1024];
+  char buf[PATH_MAX];
 
   do {
     errno = 0;
-    l = readlink(sourceP, buf, sizeof(buf));
+    l = readlink(sourceP, buf, PATH_MAX - 1);
   } while (l < 0 && errno == EINTR);
+
   if (l < 0)
     return errno;
+
   buf[l] = 0;
+
   if (symlink(buf, targetP) == 0)
     return 0;
   else if (errno != EEXIST || !repl)