Re: Unchecked setuid in Ping

[Simon Josefsson]

Jayakrishna Vadayath <jvadayat@asu.edu> writes:

> Dear Maintainer,
>
> As a part of an academic project, we have discovered situations
> inside the ping6 and ping binaries where the setuid function is
> invoked to drop privileges, however the program does not check
> if setuid has correctly dropped the privileges.
>
> This can lead to a situation where the program might execute
> code with the privileges of a higher privileged user rather than
> as a lower privileged user.
>
> The vulnerabilities lie in main @ ping/ping6.c : 255 and
> main @ ping/ping.c : 296.
>
> The documentation of setuid states
> "Note:  there  are cases where setuid() can fail even when the
> caller is UID 0; it is a grave security error to omit checking for a
> failure return from setuid()."
>
> Therefore, we feel that this is a vulnerability that must be patched.
>
> We have attached a patch file that fixes these two occurrences.
> Please fix these issues as soon as possible.

Hi Jayakrishna!  Sorry for the delay, and thanks for a perfect bug
report.  I installed it here:

https://git.savannah.gnu.org/cgit/inetutils.git/commit/?id=02a379763bf651a09b5cb728c1d6b811dc71d021

I also improved it slightly to produce useful error messages:

https://git.savannah.gnu.org/cgit/inetutils.git/commit/?id=3ce348e63c3934958a2665ef8289d28a12150948

Thanks,
Simon

>
> -- 
> Regards
> Jayakrishna Menon
> From e1c359c869184bdb361d0321c23f27704da6fcfb Mon Sep 17 00:00:00 2001
> From: Jay <jkrshnmenon@gmail.com>
> Date: Tue, 2 Jun 2020 22:12:13 -0700
> Subject: [PATCH] patching unchecked setuid in ping.c and ping6.c
>
> ---
>  ping/ping.c  | 3 ++-
>  ping/ping6.c | 3 ++-
>  2 files changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/ping/ping.c b/ping/ping.c
> index 5c3b9a47..70353f47 100644
> --- a/ping/ping.c
> +++ b/ping/ping.c
> @@ -293,7 +293,8 @@ main (int argc, char **argv)
>    ping_set_sockopt (ping, SO_BROADCAST, (char *) &one, sizeof (one));
>  
>    /* Reset root privileges */
> -  setuid (getuid ());
> +  if (setuid (getuid ()) != 0)
> +    exit (EXIT_FAILURE);
>  
>    /* Force line buffering regardless of output device.  */
>    setvbuf (stdout, NULL, _IOLBF, 0);
> diff --git a/ping/ping6.c b/ping/ping6.c
> index 8b31a9b2..aae3ff63 100644
> --- a/ping/ping6.c
> +++ b/ping/ping6.c
> @@ -252,7 +252,8 @@ main (int argc, char **argv)
>    setsockopt (ping->ping_fd, SOL_SOCKET, SO_BROADCAST, (char *) &one, sizeof 
> (one));
>  
>    /* Reset root privileges */
> -  setuid (getuid ());
> +  if (setuid (getuid ()) != 0)
> +    exit (EXIT_FAILURE);
>  
>    /* Force line buffering regardless of output device.  */
>    setvbuf (stdout, NULL, _IOLBF, 0);

signature.asc
Description: PGP signature