On 07/10/13 00:13, Denys Vlasenko wrote:
> 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 <rmal...@gmail.com>
>> ---
>>
>> 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());
Also, the setfsuid()/setfsgid() functions are Linux specific and don't
appear to be able to return errors to the caller properly. The manpage
suggests that they are no longer necessary since the signalling issues
they were introduced for in Linux no longer exist. I think it is
safe/portable to set setegid()/seteuid().
I'll post an updated patch series soon.
~Ryan
_______________________________________________
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox
