openssh unix dev - Jun 2015 - [PATCH] Fix potential use after free in uidswap.c (portable)

Fixes a potential (but probably rather unlikely) use after free bug in
function temporarily_use_uid(), file uidswap.c.

--- a/uidswap.c
+++ b/uidswap.c
@@ -113,8 +113,9 @@ temporarily_use_uid(struct passwd *pw)
                }
        }
        /* Set the effective uid to the given (unprivileged) uid. */
-       if (setgroups(user_groupslen, user_groups) < 0)
-               fatal("setgroups: %.100s", strerror(errno));
+       if (user_groupslen > 0 &&
+            (setgroups(user_groupslen, user_groups)) < 0)
+               fatal("setgroups: %.100s", strerror(errno));

Best regards,
Albert

On Sun, Jun 21, 2015 at 12:31 AM, Albert S. <mail at quitesimple.org>
wrote:
> Fixes a potential (but probably rather unlikely) use after free bug in
> function temporarily_use_uid(), file uidswap.c.
>
Does seem unlikely (with zero entries there's no reason for it to deref the
pointer), however reading the man pages it seems like there's no guarantee
that it won't, so seems reasonable to me.  Damien?

> --- a/uidswap.c
> +++ b/uidswap.c
> @@ -113,8 +113,9 @@ temporarily_use_uid(struct passwd *pw)
>                 }
>         }
>         /* Set the effective uid to the given (unprivileged) uid. */
> -       if (setgroups(user_groupslen, user_groups) < 0)
> -               fatal("setgroups: %.100s", strerror(errno));
> +       if (user_groupslen > 0 &&
> +            (setgroups(user_groupslen, user_groups)) < 0)
> +               fatal("setgroups: %.100s", strerror(errno));
>
> Best regards,
> Albert
> _______________________________________________
> openssh-unix-dev mailing list
> openssh-unix-dev at mindrot.org
> https://lists.mindrot.org/mailman/listinfo/openssh-unix-dev
>


-- 
Darren Tucker (dtucker at zip.com.au)
GPG key 8FF4FA69 / D9A3 86E9 7EEE AF4B B2D4  37C9 C982 80C7 8FF4 FA69
    Good judgement comes with experience. Unfortunately, the experience
usually comes from bad judgement.

ok djm

On Mon, 22 Jun 2015, Darren Tucker wrote:
> On Sun, Jun 21, 2015 at 12:31 AM, Albert S. <mail at quitesimple.org>
wrote:
> 
> > Fixes a potential (but probably rather unlikely) use after free bug in
> > function temporarily_use_uid(), file uidswap.c.
> >
> 
> Does seem unlikely (with zero entries there's no reason for it to deref
the
> pointer), however reading the man pages it seems like there's no
guarantee
> that it won't, so seems reasonable to me.  Damien?
> 
> 
> > --- a/uidswap.c
> > +++ b/uidswap.c
> > @@ -113,8 +113,9 @@ temporarily_use_uid(struct passwd *pw)
> >                 }
> >         }
> >         /* Set the effective uid to the given (unprivileged) uid. */
> > -       if (setgroups(user_groupslen, user_groups) < 0)
> > -               fatal("setgroups: %.100s", strerror(errno));
> > +       if (user_groupslen > 0 &&
> > +            (setgroups(user_groupslen, user_groups)) < 0)
> > +               fatal("setgroups: %.100s", strerror(errno));
> >
> > Best regards,
> > Albert
> > _______________________________________________
> > openssh-unix-dev mailing list
> > openssh-unix-dev at mindrot.org
> > https://lists.mindrot.org/mailman/listinfo/openssh-unix-dev
> >
> 
> 
> 
> -- 
> Darren Tucker (dtucker at zip.com.au)
> GPG key 8FF4FA69 / D9A3 86E9 7EEE AF4B B2D4  37C9 C982 80C7 8FF4 FA69
>     Good judgement comes with experience. Unfortunately, the experience
> usually comes from bad judgement.
> _______________________________________________
> openssh-unix-dev mailing list
> openssh-unix-dev at mindrot.org
> https://lists.mindrot.org/mailman/listinfo/openssh-unix-dev
>

On Mon, Jun 22, 2015 at 11:36 AM, Darren Tucker <dtucker at zip.com.au>
wrote:
> On Sun, Jun 21, 2015 at 12:31 AM, Albert S. <mail at quitesimple.org>
wrote:
>
>> Fixes a potential (but probably rather unlikely) use after free bug in
>> function temporarily_use_uid(), file uidswap.c.
>>
>
> Does seem unlikely (with zero entries there's no reason for it to deref
> the pointer), however reading the man pages it seems like there's no
> guarantee that it won't, so seems reasonable to me.  Damien?
>
Thanks, this has been committed and will be in the next release.

-- 
Darren Tucker (dtucker at zip.com.au)
GPG key 8FF4FA69 / D9A3 86E9 7EEE AF4B B2D4  37C9 C982 80C7 8FF4 FA69
    Good judgement comes with experience. Unfortunately, the experience
usually comes from bad judgement.