[PATCH] crontab: use O_NOFOLLOW when opening files in the spool dir

crontab is setuid root (BB_SUID_REQUIRE) and opens files in the crontab
spool directory while running with euid 0.

If the spool directory is writable by an unprivileged user, they can
create a symlink with one of the filenames used by crontab. Since the
files are currently opened without O_NOFOLLOW, the symlink is followed
and the target is modified as root.

This affects the following files:

- <user>.new is opened with O_CREAT|O_TRUNC, so a symlink can cause an
  arbitrary file to be truncated and overwritten.
- the temporary file used by -e is opened the same way.
- cron.update is opened with O_CREAT|O_APPEND, allowing data to be
  appended to an arbitrary file.

For example, a symlink can point to another user's crontab or another
root-owned file, resulting in a write-as-root primitive.

Use O_NOFOLLOW when opening these files so that a symlink as the final
path component is rejected.

There is no behavior change for a normally configured spool directory,
which is owned by root and not writable by users. Unprivileged users
can still use crontab through the setuid binary, since the file
operations are performed by the process itself.

Signed-off-by: Ali Ahmet Memis <[email protected]>
---
 miscutils/crontab.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/miscutils/crontab.c b/miscutils/crontab.c
index 1111f4d54..ae36fcca3 100644
--- a/miscutils/crontab.c
+++ b/miscutils/crontab.c
@@ -157,7 +157,8 @@ int crontab_main(int argc UNUSED_PARAM, char **argv)
 		tmp_fname = xasprintf("%s.%u", crontab_dir, (unsigned)getpid());
 		/* No O_EXCL: we don't want to be stuck if earlier crontabs
 		 * were killed, leaving stale temp file behind */
-		src_fd = xopen3(tmp_fname, O_RDWR|O_CREAT|O_TRUNC, 0600);
+		/* O_NOFOLLOW: we are root, don't follow a planted symlink */
+		src_fd = xopen3(tmp_fname, O_RDWR|O_CREAT|O_TRUNC|O_NOFOLLOW, 0600);
 		fchown(src_fd, pas->pw_uid, pas->pw_gid);
 		fd = open(pas->pw_name, O_RDONLY);
 		if (fd >= 0) {
@@ -170,12 +171,12 @@ int crontab_main(int argc UNUSED_PARAM, char **argv)
 		/* The src_fd needs to be reopened to handle editors that do
 		 * save the buffer as new file and rename it to tmp_fname (so
 		 * for example vim). */
-		src_fd = xopen3(tmp_fname, O_RDONLY, 0600);
+		src_fd = xopen3(tmp_fname, O_RDONLY|O_NOFOLLOW, 0600);
 		/* fall through */
 
 	case 0: /* Replace (no -l, -e, or -r were given) */
 		new_fname = xasprintf("%s.new", pas->pw_name);
-		fd = open(new_fname, O_WRONLY|O_CREAT|O_TRUNC|O_APPEND, 0600);
+		fd = open(new_fname, O_WRONLY|O_CREAT|O_TRUNC|O_APPEND|O_NOFOLLOW, 0600);
 		if (fd >= 0) {
 			bb_copyfd_eof(src_fd, fd);
 			close(fd);
@@ -193,7 +194,7 @@ int crontab_main(int argc UNUSED_PARAM, char **argv)
 	/* Bump notification file.  Handle window where crond picks file up
 	 * before we can write our entry out.
 	 */
-	while ((fd = open(CRONUPDATE, O_WRONLY|O_CREAT|O_APPEND, 0600)) >= 0) {
+	while ((fd = open(CRONUPDATE, O_WRONLY|O_CREAT|O_APPEND|O_NOFOLLOW, 0600)) >= 0) {
 		struct stat st;
 
 		fdprintf(fd, "%s\n", pas->pw_name);
-- 
2.55.0