On Fri, May 6, 2022 at 1:00 PM Junio C Hamano <gitster@xxxxxxxxx> wrote: > > Carlo Arenas <carenas@xxxxxxxxx> writes: > > > which is also why we can't use it, any possibly bogus or suspicious > > value we get from SUDO_UID MUST be ignored. > > I do not think I agree. If we have a strange value in SUDO_UID, it > would be much better and safer to err on the safe side. ignoring it is the safe side; for example if we replace the current function with the proposed one then some user lucky enough to have access to the latest linux supercomputer that has been patched to have a 64-bit uid_t (because who makes 32-bit supercomputers nowadays) would get root[1] access by simply faking his SUDO_UID to be UINT_MAX + 1. We will also honour probably SUDO_UID=0M as root instead of the current action which is to ignore that nonsense and most likely die by telling the pranker that he still can't run `git status` on that root owned repository he got access to even after he managed to get sudo to generate that as a SUDO_UID. > Instead of ignoring, in the situation where we care about the value > we read from SUDO_UID (i.e. when euid==0), we should die loudly when > it has a strange value. that is fair, but then it would then make this feature into a denial of service attack target ;) The current implementation instead keeps git running under the UID it was started as, which should be root if it gets to use this code under the current implementation. I am still open to changing it if you would rather let git be the last line of defense, I just think that the current implementation of ignoring it is more user friendly and better at punking would be attackers. Carlo [1] https://lwn.net/Articles/727490/
