On May 6, 2022 4:23 PM, Carlo Arenas wrote: >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. Please keep in mind the uid_t == 65535 on __TANDEM. uid_t == 0 actually means "not logged in". Thanks, Randall
