On May 9, 2022 12:55 PM, Junio C Hamano wrote: >Carlo Marcelo Arenas Belón <carenas@xxxxxxxxx> writes: > >>> In exchange, the only attack scenario the code prevents is this, >>> IIUC. >>> >>> * You, the aspiring cracker, are a user not allowed to run "sudo" on >>> the box, and you know your uid is 1000 >>> >>> * You look for another user, a potential victim, whose uid is 1000 >>> modulo 65536 (if your uid_t is 16-bit) and who can run "sudo" on >>> the box. >>> >>> * You prepare a malicious repository, invite that user there and >>> ask them to run "sudo something" there. >>> >>> I'd say such an attack vector is not likely,... > >Sorry, I was totally wrong here. > >It is not just "not likely", but such an attack, with a potential victim not futzing with >SUDO_UID environment themselves, would not work at all. The value of >SUDO_UID and the original uid of the potential victim by definition should fit in the >uid_t type. So if you, the aspiring cracker, have UID 1000, nobody else on the >system has UID that is congruent modulo uid_t and wrap-around attack does not >exist. As long as the type we use to read SUDO_UID string into a variable is not >narrower than uid_t, there. > >Of course you can tell any user who runs "sudo" to set SUDO_UID to >1000 + 64k and cause wrap-around, but then you can tell them to set SUDO_UID to >1000 without relying on wrap-around and have the same effect. So, let's stop >worrying about this bogus scenario. > >As to the "we can break compilation with -Wsign-compare on a system with signed >uid_t", I agree that is true if we have > > env_id <= (uid_t) -1 > >there. But I am not sure if that is the most effective way to smoke out platforms >where this code has trouble working correctly. Also, I would think that a system >with signed uid_t is a possibility, but a user with a negative UID? > >I do not think even nobody4 was negative ;-) I can test the code when it's ready.
