Paul Eggert <[email protected]> writes: > * With an environment variable setting like > TZ="/usr/share/zoneinfo/America/Los_Angeles", in a privileged > program
This is... imprecise. We perform additional checks if the process is setugid because it sits at a privilege boundary: it has elevated privileges but its environment is controlled by an unprivileged user. > FreeBSD first opens the directory /usr/share/zoneinfo to get > a directory file descriptor, and then uses fstatat and openat, both > with AT_RESOLVE_BENEATH. However, it passes the full name to fstatat > which must be a typo; I can't see how that would work reliably for a > setuid program, even though the neighboring code suggests it was > intended to work. I assume it was intended to pass a relative name > to fstatat. Yes, that's a typo in the code and a hole in our test suite, thank you. > * As near as I can make out, in privileged programs FreeBSD rejects > adjacent slashes in settings like > TZ="/usr/share/zoneinfo//America/Los_Angeles", where there are two > slashes after "zoneinfo". It would be nicer to treat those two > slashes as if they were one, as the kernel does. True. This is an oversight. We strip TZDIR "/" but don't strip additional slashes. > * tzcode is intended to be portable to platforms lacking openat and > AT_RESOLVE_BENEATH, so for now the attached patches mimic FreeBSD's > extra checking without using these two features. There are > efficiency motivations for using the two features if available, but > I'd like to defer that to a later patchset. This is a matter of security, not efficieny. You simply cannot get the same security guarantees without AT_RESOLVE_BENEATH. You can achieve the same result, but your code will be vulnerable to TOCTTOU races. > * Although FreeBSD takes some steps to treat TZ settings the same > regardless of whether they come from the TZ environment variable, it > doesn't take this idea to its logical conclusion. It seems to me > that it's better to not care where the TZ setting came from, as > that's easier to document, understand, and maintain, so the attached > patches do that. That's just wrong. As a setugid program, I can't trust TZ because it was provided by an unprivileged and untrusted user, so I must defend against attempts to break out from TZDIR. I should however be able to trust TZDEFAULT, even if it points to something outside TZDIR, because TZDEFAULT is controlled by the administrator, not by an untrusted user. DES -- Dag-Erling Smørgrav - [email protected]
