[PATCH 3/3] wall: Temporarily drop privileges when opening files

Tito farmatito at tiscali.it
Tue Oct 8 17:02:04 UTC 2013


On Tuesday 08 October 2013 14:55:44 Denys Vlasenko wrote:
> On Tuesday 08 October 2013 02:02, Ryan Mallon wrote:
> > The wall applet is setuid and currently does no checking of the real
> > user's read access to the message file. This allows the wall applet to
> > be used to display files which are not readable by an unprivileged
> > user. For example:
> > 
> >   $ wall /etc/shadow
> >   $ wall /proc/vmallocinfo
> > 
> > Fix this by temporarily dropping privileges before opening the file.
> 
> Applied all three patches (with small modifications).
> Thanks!
Hi,
the crontab patch seems not to be in git yet.

git pull
Updating aadb485..932e233
Fast-forward
 archival/libarchive/decompress_bunzip2.c |   26 +++++++++----
 archival/libarchive/get_header_ar.c      |   35 +++++++++++------
 archival/tar.c                           |    4 +-
 console-tools/dumpkmap.c                 |   37 +++++++++++-------
 console-tools/loadkmap.c                 |   27 +++++++++----
 coreutils/dd.c                           |  111 ++++++++++++++++++++++++++++++++++++-----------------
 coreutils/touch.c                        |   39 +++++++++++++++++--
 docs/tcp.txt                             |   21 +++++++---
 include/applets.src.h                    |    2 -
 include/libbb.h                          |    4 +-
 init/init.c                              |   35 +++++++++++------
 libbb/human_readable.c                   |    6 ++-
 libbb/lineedit.c                         |   66 +++++++++++++++++++++++--------
 libbb/unicode.c                          |    5 +--
 libbb/xfuncs_printf.c                    |   12 +++---
 miscutils/Config.src                     |    7 ----
 miscutils/Kbuild.src                     |    1 -
 miscutils/man.c                          |    2 +-
 miscutils/setsid.c                       |   14 ++++++-
 miscutils/wall.c                         |   25 +++++++++++-
 networking/httpd.c                       |   39 ++++++++++++-------
 networking/ifconfig.c                    |    2 +-
 networking/interface.c                   |    2 +-
 networking/libiproute/iplink.c           |    2 +-
 networking/udhcp/dhcpc.c                 |   49 +++++++++++------------
 procps/nmeter.c                          |    3 +-
 procps/powertop.c                        |    3 +-
 procps/ps.c                              |    9 ++---
 procps/top.c                             |    3 +-
 scripts/trylink                          |    6 +--
 util-linux/fdisk_gpt.c                   |    8 ++--
 util-linux/swaponoff.c                   |   14 +++++++
 32 files changed, 412 insertions(+), 207 deletions(-)

Is there any particular reason we use setfsgid/setfsuid that are
"Linux-specific and should not be used in programs intended to be portable."

I just checked in a older android bionic libc and the calls are not there
so sooner or later the android or BSD people will start to complain.
I also checked the latest util-linux tar 2.24-rc1 and in wall they do:

if (fname) {
			/*
			 * When we are not root, but suid or sgid, refuse to read files
			 * (e.g. device files) that the user may not have access to.
			 * After all, our invoker can easily do "wall < file"
			 * instead of "wall file".
			 */
			uid_t uid = getuid();
			if (uid && (uid != geteuid() || getgid() != getegid()))
				errx(EXIT_FAILURE, _("will not read %s - use stdin."),
				     fname);
			if (!freopen(fname, "r", stdin))
				err(EXIT_FAILURE, _("cannot open %s"), fname);

		}
Couldn't we use a similar solution?

I also have some troubles making wall working correctly in my xterm:

./busybox wall
prova
wall: can't open '/dev/:0': No such file or directory

ls -la busybox
-rwsr-sr-x 1 root root 753004 Oct  8 18:30 busybox

but adding this 2 lines fixes it:

	while ((ut = getutent()) != NULL) {
		char *line;
		if (ut->ut_type != USER_PROCESS)
			continue;
+		if (ut->ut_line[0] == ':')
+			continue;
		line = concat_path_file("/dev", ut->ut_line);
		xopen_xwrite_close(line, msg);
		free(line);
	}

Comment in wall of util-linux  2.24-rc1 says:

/* Joey Hess reports that use-sessreg in /etc/X11/wdm/
		   produces ut_line entries like :0, and a write
		   to /dev/:0 fails. */

Cannot say if it is worth to add them to busybox.

Ciao,
Tito


More information about the busybox mailing list