From: Phillip Wood Date: Wed, 27 Apr 2022 17:28:03 GMT Subject: Re: [PATCH] git-compat-util: avoid failing dir ownership checks if running privileged Message-ID: <6c3b447d-9419-982d-678c-6a6c0ff8cbc2@gmail.com> In-Reply-To: On 27/04/2022 17:54, Carlo Arenas wrote: > On Wed, Apr 27, 2022 at 9:31 AM Phillip Wood wrote: >> 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) If we think uid_t is a uint32_t then should we be using strtoul() to make sure we cover the whole uid range where sizeof(long) == sizeof(uint32_t)? Best Wishes Phillip > Carlo