On Monday 30 September 2013 00:30, 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 issue by introducing the same check as used by the util-linux
> version of wall: only allow the file argument to be used if the user is root,
> or the real and effective uid/gids are equal.
>
> Note, some files in /proc, etc do have global read permission, but may have
> different contents if read as root (for example Linux's kptr_restrict
> feature). User's may still run:
>
> $ wall < file
>
> If they do legitimately have read access to some file.
>
> Signed-off-by: Ryan Mallon <[email protected]>
> ---
>
> diff --git a/miscutils/wall.c b/miscutils/wall.c
> index 762f53b..f0a6cd4 100644
> --- a/miscutils/wall.c
> +++ b/miscutils/wall.c
> @@ -23,6 +23,20 @@ int wall_main(int argc UNUSED_PARAM, char **argv)
> struct utmp *ut;
> char *msg;
> int fd = argv[1] ? xopen(argv[1], O_RDONLY) : STDIN_FILENO;
> + uid_t ruid;
> +
> + /*
> + * If we are not root, but are setuid or setgid, then don't allow
> + * reading of any files the real uid might not have access to. The
> + * user can do 'wall < file' instead of 'wall file' if this disallows
> + * access to some legimate file.
> + */
> + ruid = getuid();
> + if (fd != STDIN_FILENO && ruid != 0 &&
> + (ruid != geteuid() || getgid() != getegid())) {
> + bb_error_msg_and_die("will not read %s - use stdin or '%s <
> %s'",
> + argv[1], argv[0], argv[1]);
> + }
>
> msg = xmalloc_read(fd, NULL);
> if (ENABLE_FEATURE_CLEAN_UP && argv[1])
How about the following patch? -
--- busybox.3/miscutils/wall.c 2013-07-25 03:48:31.000000000 +0200
+++ busybox.4/miscutils/wall.c 2013-10-06 15:01:55.000000000 +0200
@@ -22,8 +34,19 @@ int wall_main(int argc UNUSED_PARAM, cha
{
struct utmp *ut;
char *msg;
- int fd = argv[1] ? xopen(argv[1], O_RDONLY) : STDIN_FILENO;
+ int fd;
+ fd = STDIN_FILENO;
+ if (argv[1]) {
+ /* The applet is setuid.
+ * Access to the file must be under user's uid/gid.
+ */
+ setfsuid(getuid());
+ setfsgid(getgid());
+ fd = xopen(argv[1], O_RDONLY);
+ setfsuid(geteuid());
+ setfsgid(getegid());
+ }
msg = xmalloc_read(fd, NULL);
if (ENABLE_FEATURE_CLEAN_UP && argv[1])
close(fd);
This way, kernel will do access checks for us.
_______________________________________________
busybox mailing list
[email protected]
http://lists.busybox.net/mailman/listinfo/busybox
