From 56b53a30a15622e366e425f0a44c1ff81ef76e30 Mon Sep 17 00:00:00 2001 From: Jon Trulson Date: Tue, 10 Apr 2018 19:05:56 -0600 Subject: [PATCH] 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. --- cde/programs/dtfile/Directory.c | 8 ++++---- cde/programs/dtfile/SharedProcs.c | 4 ++-- cde/programs/dtfile/dtcopy/fsrtns.c | 7 +++++-- 3 files changed, 11 insertions(+), 8 deletions(-) diff --git a/cde/programs/dtfile/Directory.c b/cde/programs/dtfile/Directory.c index 578e92660..618a4b9cc 100644 --- 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 48a21aff5..c823b4320 100644 --- 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 5532aca2a..fb29f0093 100644 --- 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)