Re: git: b6ea2513f776 - main - tzcode: Limit TZ for setugid programs
Date: Thu, 21 Aug 2025 22:25:45 UTC
In message <202508211859.57LIxlcm003786@gitrepo.freebsd.org>, Dag-Erling
=?utf-
8?Q?Sm=C3=B8rgrav?= writes:
> The branch main has been updated by des:
>
> URL: https://cgit.FreeBSD.org/src/commit/?id=b6ea2513f7769ea9d9e4d342777111ad
> d2c903b0
>
> commit b6ea2513f7769ea9d9e4d342777111add2c903b0
> Author: Dag-Erling Smørgrav <des@FreeBSD.org>
> AuthorDate: 2025-08-21 16:34:32 +0000
> Commit: Dag-Erling Smørgrav <des@FreeBSD.org>
> CommitDate: 2025-08-21 18:59:37 +0000
>
> tzcode: Limit TZ for setugid programs
>
> The zoneinfo parser can be told to read any file the program can access
> by setting TZ to either an absolute path, or a path relative to the
> zoneinfo directory. For setugid programs, we previously had a hack from
> OpenBSD which rejects values of TZ deemed unsafe, but that was rather
> arbitrary (anything containing a dot, for instance). Leverage openat()
> with AT_RESOLVE_BENEATH instead.
>
> For simplicity, move the TZ change detection code to after we've opened
> the file, and stat the file descriptor rather than the name.
>
> Reviewed by: jhb
> Differential Revision: https://reviews.freebsd.org/D52029
> ---
> contrib/tzcode/localtime.c | 70 +++++++++++++++++++++++++++++---------------
> --
> 1 file changed, 45 insertions(+), 25 deletions(-)
>
> diff --git a/contrib/tzcode/localtime.c b/contrib/tzcode/localtime.c
> index 6eabe0afe570..1a01db931cab 100644
> --- a/contrib/tzcode/localtime.c
> +++ b/contrib/tzcode/localtime.c
> @@ -408,13 +408,13 @@ scrub_abbrs(struct state *sp)
> * 1 if the time zone has changed
> */
> static int
> -tzfile_changed(const char *name)
> +tzfile_changed(const char *name, int fd)
> {
> static char old_name[PATH_MAX];
> static struct stat old_sb;
> struct stat sb;
>
> - if (stat(name, &sb) != 0)
> + if (_fstat(fd, &sb) != 0)
> return -1;
>
> if (strcmp(name, old_name) != 0) {
> @@ -446,8 +446,10 @@ union input_buffer {
> + 4 * TZ_MAX_TIMES];
> };
>
> +#ifndef __FreeBSD__
> /* TZDIR with a trailing '/' rather than a trailing '\0'. */
> static char const tzdirslash[sizeof TZDIR] = TZDIR "/";
> +#endif /* !__FreeBSD__ */
>
> /* Local storage needed for 'tzloadbody'. */
> union local_storage {
> @@ -460,6 +462,7 @@ union local_storage {
> struct state st;
> } u;
>
> +#ifndef __FreeBSD__
> /* The name of the file to be opened. Ideally this would have no
> size limits, to support arbitrarily long Zone names.
> Limiting Zone names to 1024 bytes should suffice for practical use.
> @@ -467,6 +470,7 @@ union local_storage {
> file_analysis as that struct is allocated anyway, as the other
> union member. */
> char fullname[max(sizeof(struct file_analysis), sizeof tzdirslash + 1024)]
> ;
> +#endif /* !__FreeBSD__ */
> };
>
> /* Load tz data from the file named NAME into *SP. Read extended
> @@ -480,7 +484,11 @@ tzloadbody(char const *name, struct state *sp, bool doex
> tend,
> register int fid;
> register int stored;
> register ssize_t nread;
> +#ifdef __FreeBSD__
> + int serrno;
> +#else /* !__FreeBSD__ */
> register bool doaccess;
> +#endif /* !__FreeBSD__ */
> register union input_buffer *up = &lsp->u.u;
> register int tzheadsize = sizeof(struct tzhead);
>
> @@ -494,6 +502,7 @@ tzloadbody(char const *name, struct state *sp, bool doext
> end,
>
> if (name[0] == ':')
> ++name;
> +#ifndef __FreeBSD__
> #ifdef SUPPRESS_TZDIR
> /* Do not prepend TZDIR. This is intended for specialized
> applications only, due to its security implications. */
> @@ -502,9 +511,7 @@ tzloadbody(char const *name, struct state *sp, bool doext
> end,
> doaccess = name[0] == '/';
> #endif
> if (!doaccess) {
> -#ifndef __FreeBSD__
> char const *dot;
> -#endif /* !__FreeBSD__ */
> if (sizeof lsp->fullname - sizeof tzdirslash <= strlen(name))
> return ENAMETOOLONG;
>
> @@ -514,7 +521,6 @@ tzloadbody(char const *name, struct state *sp, bool doext
> end,
> memcpy(lsp->fullname, tzdirslash, sizeof tzdirslash);
> strcpy(lsp->fullname + sizeof tzdirslash, name);
>
> -#ifndef __FreeBSD__
> /* Set doaccess if NAME contains a ".." file name
> component, as such a name could read a file outside
> the TZDIR virtual subtree. */
> @@ -524,35 +530,49 @@ tzloadbody(char const *name, struct state *sp, bool doe
> xtend,
> doaccess = true;
> break;
> }
> -#endif /* !__FreeBSD__ */
>
> name = lsp->fullname;
> }
> -#ifndef __FreeBSD__
> if (doaccess && access(name, R_OK) != 0)
> return errno;
> -#endif /* !__FreeBSD__ */
> -#ifdef DETECT_TZ_CHANGES
> - if (doextend) {
> - /*
> - * Detect if the timezone file has changed. Check
> - * 'doextend' to ignore TZDEFRULES; the tzfile_changed()
> - * function can only keep state for a single file.
> - */
> - switch (tzfile_changed(name)) {
> - case -1:
> - return errno;
> - case 0:
> - return 0;
> - case 1:
> - break;
> - }
> - }
> -#endif /* DETECT_TZ_CHANGES */
> +#else /* __FreeBSD__ */
> + if (issetugid()) {
> + const char *relname = name;
> + if (strncmp(relname, TZDIR "/", strlen(TZDIR) + 1) == 0)
> + relname += strlen(TZDIR) + 1;
> + int dd = _open(TZDIR, O_DIRECTORY | O_RDONLY);
> + if (dd < 0)
> + return errno;
> + fid = _openat(dd, relname, O_RDONLY | O_BINARY, AT_RESOLVE_BENEATH
> );
> + serrno = errno;
> + _close(dd);
> + errno = serrno;
> + } else
> +#endif
> fid = _open(name, O_RDONLY | O_BINARY);
> if (fid < 0)
> return errno;
>
> +#ifdef DETECT_TZ_CHANGES
> + if (doextend) {
> + /*
> + * Detect if the timezone file has changed. Check 'doextend' to
> + * ignore TZDEFRULES; the tzfile_changed() function can only
> + * keep state for a single file.
> + */
> + switch (tzfile_changed(name, fid)) {
> + case -1:
> + serrno = errno;
> + _close(fid);
> + return serrno;
> + case 0:
> + _close(fid);
> + return 0;
> + case 1:
> + break;
> + }
> + }
> +#endif /* DETECT_TZ_CHANGES */
> nread = _read(fid, up->buf, sizeof up->buf);
> if (nread < tzheadsize) {
> int err = nread < 0 ? errno : EINVAL;
>
Further testing has shown that this commit has TZ America/Vancouver to UTC.
Setting TZ to PST8PDT still works but TZ=America/Vancouver does not.
--
Cheers,
Cy Schubert <Cy.Schubert@cschubert.com>
FreeBSD UNIX: <cy@FreeBSD.org> Web: https://FreeBSD.org
NTP: <cy@nwtime.org> Web: https://nwtime.org
e**(i*pi)+1=0