From a273e09cb25590c3968ff924a516c99b90c33a5b Mon Sep 17 00:00:00 2001
From: Mark Johnston <markj@FreeBSD.org>
Date: Wed, 27 Mar 2019 19:40:18 +0000
Subject: [PATCH] Fix pidfile_open(3) to handle relative paths with multiple
 components.

r322369's use of basename(3) was incorrect and worked by accident so
long as the pidfile path was absolute or consisted of a single
component.  Fix the basename() usage and add a regression test.

Reported by:	0mp
Reviewed by:	cem
MFC after:	3 days
Differential Revision:	https://reviews.freebsd.org/D19728
---
 lib/libutil/pidfile.c            | 22 ++++++++++++--------
 lib/libutil/tests/pidfile_test.c | 35 ++++++++++++++++++++++++++++++++
 2 files changed, 48 insertions(+), 9 deletions(-)

diff --git a/lib/libutil/pidfile.c b/lib/libutil/pidfile.c
index b0ce2f30d2d..05f8174bb1d 100644
--- a/lib/libutil/pidfile.c
+++ b/lib/libutil/pidfile.c
@@ -100,8 +100,9 @@ pidfile_read(int dirfd, const char *filename, pid_t *pidptr)
 }
 
 struct pidfh *
-pidfile_open(const char *path, mode_t mode, pid_t *pidptr)
+pidfile_open(const char *pathp, mode_t mode, pid_t *pidptr)
 {
+	char path[MAXPATHLEN];
 	struct pidfh *pfh;
 	struct stat sb;
 	int error, fd, dirfd, dirlen, filenamelen, count;
@@ -112,19 +113,22 @@ pidfile_open(const char *path, mode_t mode, pid_t *pidptr)
 	if (pfh == NULL)
 		return (NULL);
 
-	if (path == NULL) {
+	if (pathp == NULL) {
 		dirlen = snprintf(pfh->pf_dir, sizeof(pfh->pf_dir),
 		    "/var/run/");
 		filenamelen = snprintf(pfh->pf_filename,
 		    sizeof(pfh->pf_filename), "%s.pid", getprogname());
 	} else {
-		dirlen = snprintf(pfh->pf_dir, sizeof(pfh->pf_dir),
-		    "%s", path);
-		filenamelen = snprintf(pfh->pf_filename,
-		    sizeof(pfh->pf_filename), "%s", path);
-
-		dirname(pfh->pf_dir);
-		basename(pfh->pf_filename);
+		if (strlcpy(path, pathp, sizeof(path)) >= sizeof(path)) {
+			free(pfh);
+			errno = ENAMETOOLONG;
+			return (NULL);
+		}
+		dirlen = strlcpy(pfh->pf_dir, dirname(path),
+		    sizeof(pfh->pf_dir));
+		(void)strlcpy(path, pathp, sizeof(path));
+		filenamelen = strlcpy(pfh->pf_filename, basename(path),
+		    sizeof(pfh->pf_filename));
 	}
 
 	if (dirlen >= (int)sizeof(pfh->pf_dir) ||
diff --git a/lib/libutil/tests/pidfile_test.c b/lib/libutil/tests/pidfile_test.c
index 42a7be4db84..9e516c35273 100644
--- a/lib/libutil/tests/pidfile_test.c
+++ b/lib/libutil/tests/pidfile_test.c
@@ -263,6 +263,40 @@ test_pidfile_inherited(void)
 	return (result);
 }
 
+/*
+ * Make sure we handle relative pidfile paths correctly.
+ */
+static const char *
+test_pidfile_relative(void)
+{
+	char path[PATH_MAX], pid[32], tmpdir[PATH_MAX];
+	struct pidfh *pfh;
+	int fd;
+
+	(void)snprintf(tmpdir, sizeof(tmpdir), "%s.XXXXXX", __func__);
+	if (mkdtemp(tmpdir) == NULL)
+		return (strerror(errno));
+	(void)snprintf(path, sizeof(path), "%s/pidfile", tmpdir);
+
+	pfh = pidfile_open(path, 0600, NULL);
+	if (pfh == NULL)
+		return (strerror(errno));
+	if (pidfile_write(pfh) != 0)
+		return (strerror(errno));
+	fd = open(path, O_RDONLY);
+	if (fd < 0)
+		return (strerror(errno));
+	if (read(fd, pid, sizeof(pid)) < 0)
+		return (strerror(errno));
+	if (atoi(pid) != getpid())
+		return ("pid mismatch");
+	if (close(fd) != 0)
+		return (strerror(errno));
+	if (pidfile_close(pfh) != 0)
+		return (strerror(errno));
+	return (NULL);
+}
+
 static struct test {
 	const char *name;
 	const char *(*func)(void);
@@ -271,6 +305,7 @@ static struct test {
 	{ "pidfile_self", test_pidfile_self },
 	{ "pidfile_contested", test_pidfile_contested },
 	{ "pidfile_inherited", test_pidfile_inherited },
+	{ "pidfile_relative", test_pidfile_relative },
 };
 
 int