Re: [PATCH] git-compat-util: avoid failing dir ownership checks if running privileged

From: Carlo Arenas <carenas@gmail.com>
Date: Apr 27, 2022, 16:54 UTC
Message-ID: <CAPUEsphumEJ=qBM9c9WLJ6fzL=mcK2+3fUViyHEvttaD-DsOgg@mail.gmail.com>
In-Reply-To: <57217c15-c6d1-32f1-a481-0003ba66e342@gmail.com>

On Wed, Apr 27, 2022 at 9:31 AM Phillip Wood <phillip.wood123@gmail.com> wrote:

Show 11 quoted lines

> On 27/04/2022 16:38, Carlo Arenas wrote:
> > FWIW, I still think that using atoi with a check to skip "" is
> > probably as safe as doing all this extra checking as no one has shown
> > yet a system where sizeof(uid_t) > sizeof(uint32_t), but agree with
> > Junio that using long instead avoids issues with the systems where
> > sizeof(uid_t) > sizeof(int) and unless sizeof(int) == sizeof(long)
> > (ex: 32-bit Linux) which is then covered by the cast.
>
> if sizeof(uid_t) < sizeof(long) then the cast will truncate the value
> returned by strtol() which means we are trusting that SUDO_UID is a
> valid uid otherwise it will be truncated.

correct, this whole procedure relies on the fact that SUDO_UID is not a bogus value (ex: it was produced by a non buggy sudo and hasn't been tampered with)

in systems where sizeof(uid_t) < sizeof(long), it is expected that the id we got should be able to fit in an uid_t so no truncation will ever happen.

the only thing that worries me is sign extension but that is why I put a specific cast. for all practical reasons I expect uid_t to be uint32_t and therefore using long should be better than using int (through atoi)

Carlo