[PATCH] add/remove-shell: prevent world writable /etc/shells

Natanael Copa ncopa at alpinelinux.org
Wed May 10 16:40:19 UTC 2017


add-shell will not preserve the current permissions, and if umask is 0
it will create the /etc/shells world writable. To reproduce:

  umask 0; add-shell /bin/bash; ls -l /etc/shells

As a workaround we add the current st_mode with xopen3, which at least
will prevent /etc/shells to get more permissions than it previously
had.

Signed-off-by: Natanael Copa <ncopa at alpinelinux.org>
---
 loginutils/add-remove-shell.c | 11 +++--------
 1 file changed, 3 insertions(+), 8 deletions(-)

diff --git a/loginutils/add-remove-shell.c b/loginutils/add-remove-shell.c
index af7c31779..a434d054d 100644
--- a/loginutils/add-remove-shell.c
+++ b/loginutils/add-remove-shell.c
@@ -54,6 +54,7 @@ int add_remove_shell_main(int argc UNUSED_PARAM, char **argv)
 	FILE *orig_fp;
 	char *orig_fn;
 	char *new_fn;
+	struct stat sb;
 
 	argv++;
 
@@ -63,6 +64,7 @@ int add_remove_shell_main(int argc UNUSED_PARAM, char **argv)
 	orig_fp = fopen_for_read(orig_fn);
 
 	new_fn = xasprintf("%s.tmp", orig_fn);
+	xfstat(fileno(orig_fp), &sb, orig_fn);
 	/*
 	 * O_TRUNC or O_EXCL? At the first glance, O_EXCL looks better,
 	 * since it prevents races. But: (1) it requires a retry loop,
@@ -71,14 +73,7 @@ int add_remove_shell_main(int argc UNUSED_PARAM, char **argv)
 	 * after which it should revert to O_TRUNC.
 	 * For now, I settle for O_TRUNC instead.
 	 */
-	xmove_fd(xopen(new_fn, O_WRONLY | O_CREAT | O_TRUNC), STDOUT_FILENO);
-
-	/* TODO:
-	struct stat sb;
-	xfstat(fileno(orig_fp), &sb);
-	xfchown(STDOUT_FILENO, sb.st_uid, sb.st_gid);
-	xfchmod(STDOUT_FILENO, sb.st_mode);
-	*/
+	xmove_fd(xopen3(new_fn, O_WRONLY | O_CREAT | O_TRUNC, sb.st_mode), STDOUT_FILENO);
 
 	if (orig_fp) {
 		/* Copy old file, possibly skipping removed shell names */
-- 
2.12.2



More information about the busybox mailing list