The first difference between FreeBSD and tzcode localtime.c that I'd
like to address is that FreeBSD is more cautious about handling
outlandish TZ settings like "../foo". It seems to me that tzcode should
largely mimic FreeBSD's extra caution. In the long run I'd like tzcode
localtime.c to be byte-for-byte identical to FreeBSD localtime.c but
it's better to do this one step at a time and this security caution is a
good first step.
I noticed some minor issues with the FreeBSD implementation of this
extra checking, so I propose implementing it in a somewhat different way
in tzcode. I'm attaching proposed patches to do that, and have installed
them in the tzcode development repository on GitHub.
Here are issues I noticed:
* With an environment variable setting like
TZ="/usr/share/zoneinfo/America/Los_Angeles", in a privileged program
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.
* 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.
* 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.
* 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.
* FreeBSD uses issetugid, which is not universally available. The
attached patches work around this by supplying an issetugid substitute
for platforms like GNU/Linux that lack it.
The first attached patch is a minor indenting fixup. The second one is a
minor efficiency improvement, to use strnlen instead of strlen when that
improves asymptotic efficiency. The third patch adopts FreeBSD's idea of
treating a TZ setting starting with "/usr/share/zoneinfo/" as if it
lacked that prefix. And the fourth and main patch changes the tzcode
security checks as described above.
These tzcode patches do not address other differences between tzcode and
FreeBSD, such as the multithreading differences or the check-for-changes
differences. These can wait for later patchsets.
Comments and suggestions are welcome as usual.
From 07f7f31ac906609696e28647d73bc4411cc4f3b3 Mon Sep 17 00:00:00 2001
From: Paul Eggert <[email protected]>
Date: Wed, 24 Sep 2025 09:40:29 -0700
Subject: [PROPOSED 1/4] Fix preprocessor indenting
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
* localtime.c, private.h, tzfile.h, zic.c:
Fix indenting glitches uncovered by cppi.
However, do not fix the top-level “glitch” caused by
‘#ifndef XXX_H ... #define XXX_H ... #endif /* XXX_H */’
around an entire include file, as that would add an annoying extra
level of indentation that is unnecessary for reading clarity.
---
localtime.c | 14 ++--
private.h | 222 ++++++++++++++++++++++++++--------------------------
tzfile.h | 2 +-
zic.c | 4 +-
4 files changed, 121 insertions(+), 121 deletions(-)
diff --git a/localtime.c b/localtime.c
index b32d475b..378694fb 100644
--- a/localtime.c
+++ b/localtime.c
@@ -82,16 +82,16 @@ static_assert(IINNTT_MIN < INT_MIN && INT_MAX < IINNTT_MAX);
# define time_t timex_t
# if MKTIME_FITS_IN(LONG_MIN, LONG_MAX)
typedef long timex_t;
-# define TIME_T_MIN LONG_MIN
-# define TIME_T_MAX LONG_MAX
+# define TIME_T_MIN LONG_MIN
+# define TIME_T_MAX LONG_MAX
# elif MKTIME_FITS_IN(LLONG_MIN, LLONG_MAX)
typedef long long timex_t;
-# define TIME_T_MIN LLONG_MIN
-# define TIME_T_MAX LLONG_MAX
+# define TIME_T_MIN LLONG_MIN
+# define TIME_T_MAX LLONG_MAX
# else
typedef intmax_t timex_t;
-# define TIME_T_MIN INTMAX_MIN
-# define TIME_T_MAX INTMAX_MAX
+# define TIME_T_MIN INTMAX_MIN
+# define TIME_T_MAX INTMAX_MAX
# endif
# ifdef TM_GMTOFF
@@ -296,7 +296,7 @@ static int lcl_is_set;
# if SUPPORT_C89
static struct tm tm;
-#endif
+# endif
# if 2 <= HAVE_TZNAME + TZ_TIME_T
char *tzname[2] = { UNCONST(wildabbr), UNCONST(wildabbr) };
diff --git a/private.h b/private.h
index 18fdf797..b2856e2e 100644
--- a/private.h
+++ b/private.h
@@ -322,150 +322,150 @@
** previously included files. glibc 2.1 and Solaris 10 and later have
** stdint.h, even with pre-C99 compilers.
*/
-#if !defined HAVE_STDINT_H && defined __has_include
-# define HAVE_STDINT_H 1 /* C23 __has_include implies C99 stdint.h. */
-#endif
-#ifndef HAVE_STDINT_H
-# define HAVE_STDINT_H \
- (199901 <= __STDC_VERSION__ \
- || 2 < __GLIBC__ + (1 <= __GLIBC_MINOR__) \
- || __CYGWIN__ || INTMAX_MAX)
-#endif /* !defined HAVE_STDINT_H */
-
-#if HAVE_STDINT_H
-# include <stdint.h>
-#endif /* !HAVE_STDINT_H */
-
-#ifndef HAVE_INTTYPES_H
-# define HAVE_INTTYPES_H HAVE_STDINT_H
-#endif
-#if HAVE_INTTYPES_H
-# include <inttypes.h>
-#endif
-
-/* Pre-C99 GCC compilers define __LONG_LONG_MAX__ instead of LLONG_MAX. */
-#if defined __LONG_LONG_MAX__ && !defined __STRICT_ANSI__
-# ifndef LLONG_MAX
-# define LLONG_MAX __LONG_LONG_MAX__
+# if !defined HAVE_STDINT_H && defined __has_include
+# define HAVE_STDINT_H 1 /* C23 __has_include implies C99 stdint.h. */
# endif
-# ifndef LLONG_MIN
-# define LLONG_MIN (-1 - LLONG_MAX)
+# ifndef HAVE_STDINT_H
+# define HAVE_STDINT_H \
+ (199901 <= __STDC_VERSION__ \
+ || 2 < __GLIBC__ + (1 <= __GLIBC_MINOR__) \
+ || __CYGWIN__ || INTMAX_MAX)
+# endif /* !defined HAVE_STDINT_H */
+
+# if HAVE_STDINT_H
+# include <stdint.h>
+# endif /* !HAVE_STDINT_H */
+
+# ifndef HAVE_INTTYPES_H
+# define HAVE_INTTYPES_H HAVE_STDINT_H
# endif
-# ifndef ULLONG_MAX
-# define ULLONG_MAX (LLONG_MAX * 2ull + 1)
+# if HAVE_INTTYPES_H
+# include <inttypes.h>
# endif
-#endif
-#ifndef INT_FAST64_MAX
-# if 1 <= LONG_MAX >> 31 >> 31
+/* Pre-C99 GCC compilers define __LONG_LONG_MAX__ instead of LLONG_MAX. */
+# if defined __LONG_LONG_MAX__ && !defined __STRICT_ANSI__
+# ifndef LLONG_MAX
+# define LLONG_MAX __LONG_LONG_MAX__
+# endif
+# ifndef LLONG_MIN
+# define LLONG_MIN (-1 - LLONG_MAX)
+# endif
+# ifndef ULLONG_MAX
+# define ULLONG_MAX (LLONG_MAX * 2ull + 1)
+# endif
+# endif
+
+# ifndef INT_FAST64_MAX
+# if 1 <= LONG_MAX >> 31 >> 31
typedef long int_fast64_t;
-# define INT_FAST64_MIN LONG_MIN
-# define INT_FAST64_MAX LONG_MAX
-# else
+# define INT_FAST64_MIN LONG_MIN
+# define INT_FAST64_MAX LONG_MAX
+# else
/* If this fails, compile with -DHAVE_STDINT_H or with a better compiler. */
typedef long long int_fast64_t;
-# define INT_FAST64_MIN LLONG_MIN
-# define INT_FAST64_MAX LLONG_MAX
+# define INT_FAST64_MIN LLONG_MIN
+# define INT_FAST64_MAX LLONG_MAX
+# endif
# endif
-#endif
-#ifndef PRIdFAST64
-# if INT_FAST64_MAX == LONG_MAX
-# define PRIdFAST64 "ld"
-# else
-# define PRIdFAST64 "lld"
+# ifndef PRIdFAST64
+# if INT_FAST64_MAX == LONG_MAX
+# define PRIdFAST64 "ld"
+# else
+# define PRIdFAST64 "lld"
+# endif
# endif
-#endif
-#ifndef SCNdFAST64
-# define SCNdFAST64 PRIdFAST64
-#endif
+# ifndef SCNdFAST64
+# define SCNdFAST64 PRIdFAST64
+# endif
-#ifndef INT_FAST32_MAX
-# if INT_MAX >> 31 == 0
+# ifndef INT_FAST32_MAX
+# if INT_MAX >> 31 == 0
typedef long int_fast32_t;
-# define INT_FAST32_MAX LONG_MAX
-# define INT_FAST32_MIN LONG_MIN
-# else
+# define INT_FAST32_MAX LONG_MAX
+# define INT_FAST32_MIN LONG_MIN
+# else
typedef int int_fast32_t;
-# define INT_FAST32_MAX INT_MAX
-# define INT_FAST32_MIN INT_MIN
+# define INT_FAST32_MAX INT_MAX
+# define INT_FAST32_MIN INT_MIN
+# endif
# endif
-#endif
-#ifndef INT_LEAST32_MAX
+# ifndef INT_LEAST32_MAX
typedef int_fast32_t int_least32_t;
-#endif
+# endif
-#ifndef INTMAX_MAX
-# ifdef LLONG_MAX
+# ifndef INTMAX_MAX
+# ifdef LLONG_MAX
typedef long long intmax_t;
-# ifndef HAVE_STRTOLL
-# define HAVE_STRTOLL 1
+# ifndef HAVE_STRTOLL
+# define HAVE_STRTOLL 1
+# endif
+# if HAVE_STRTOLL
+# define strtoimax strtoll
+# endif
+# define INTMAX_MAX LLONG_MAX
+# define INTMAX_MIN LLONG_MIN
+# else
+typedef long intmax_t;
+# define INTMAX_MAX LONG_MAX
+# define INTMAX_MIN LONG_MIN
# endif
-# if HAVE_STRTOLL
-# define strtoimax strtoll
+# ifndef strtoimax
+# define strtoimax strtol
# endif
-# define INTMAX_MAX LLONG_MAX
-# define INTMAX_MIN LLONG_MIN
-# else
-typedef long intmax_t;
-# define INTMAX_MAX LONG_MAX
-# define INTMAX_MIN LONG_MIN
# endif
-# ifndef strtoimax
-# define strtoimax strtol
-# endif
-#endif
-#ifndef PRIdMAX
-# if INTMAX_MAX == LLONG_MAX
-# define PRIdMAX "lld"
-# else
-# define PRIdMAX "ld"
+# ifndef PRIdMAX
+# if INTMAX_MAX == LLONG_MAX
+# define PRIdMAX "lld"
+# else
+# define PRIdMAX "ld"
+# endif
# endif
-#endif
-#ifndef PTRDIFF_MAX
-# define PTRDIFF_MAX MAXVAL(ptrdiff_t, TYPE_BIT(ptrdiff_t))
-#endif
+# ifndef PTRDIFF_MAX
+# define PTRDIFF_MAX MAXVAL(ptrdiff_t, TYPE_BIT(ptrdiff_t))
+# endif
-#ifndef UINT_FAST32_MAX
+# ifndef UINT_FAST32_MAX
typedef unsigned long uint_fast32_t;
-#endif
+# endif
-#ifndef UINT_FAST64_MAX
-# if 3 <= ULONG_MAX >> 31 >> 31
+# ifndef UINT_FAST64_MAX
+# if 3 <= ULONG_MAX >> 31 >> 31
typedef unsigned long uint_fast64_t;
-# define UINT_FAST64_MAX ULONG_MAX
-# else
+# define UINT_FAST64_MAX ULONG_MAX
+# else
/* If this fails, compile with -DHAVE_STDINT_H or with a better compiler. */
typedef unsigned long long uint_fast64_t;
-# define UINT_FAST64_MAX ULLONG_MAX
+# define UINT_FAST64_MAX ULLONG_MAX
+# endif
# endif
-#endif
-#ifndef UINTMAX_MAX
-# ifdef ULLONG_MAX
+# ifndef UINTMAX_MAX
+# ifdef ULLONG_MAX
typedef unsigned long long uintmax_t;
-# define UINTMAX_MAX ULLONG_MAX
-# else
+# define UINTMAX_MAX ULLONG_MAX
+# else
typedef unsigned long uintmax_t;
-# define UINTMAX_MAX ULONG_MAX
+# define UINTMAX_MAX ULONG_MAX
+# endif
# endif
-#endif
-#ifndef PRIuMAX
-# ifdef ULLONG_MAX
-# define PRIuMAX "llu"
-# else
-# define PRIuMAX "lu"
+# ifndef PRIuMAX
+# ifdef ULLONG_MAX
+# define PRIuMAX "llu"
+# else
+# define PRIuMAX "lu"
+# endif
# endif
-#endif
-#ifndef SIZE_MAX
-# define SIZE_MAX ((size_t) -1)
-#endif
+# ifndef SIZE_MAX
+# define SIZE_MAX ((size_t) -1)
+# endif
#endif /* PORT_TO_C89 */
@@ -744,10 +744,10 @@ typedef time_tz tz_time_t;
# endif
DEPRECATED_IN_C23 char *asctime(struct tm const *);
DEPRECATED_IN_C23 char *ctime(time_t const *);
-#if SUPPORT_POSIX2008
+# if SUPPORT_POSIX2008
char *asctime_r(struct tm const *restrict, char *restrict);
char *ctime_r(time_t const *, char *);
-#endif
+# endif
ATTRIBUTE_CONST double difftime(time_t, time_t);
size_t strftime(char *restrict, size_t, char const *restrict,
struct tm const *restrict);
@@ -1008,9 +1008,9 @@ time_t timeoff(struct tm *, long);
*/
#if HAVE_GETTEXT
-#define _(msgid) gettext(msgid)
+# define _(msgid) gettext(msgid)
#else /* !HAVE_GETTEXT */
-#define _(msgid) msgid
+# define _(msgid) msgid
#endif /* !HAVE_GETTEXT */
#if !defined TZ_DOMAIN && defined HAVE_GETTEXT
@@ -1018,8 +1018,8 @@ time_t timeoff(struct tm *, long);
#endif
#if HAVE_INCOMPATIBLE_CTIME_R
-#undef asctime_r
-#undef ctime_r
+# undef asctime_r
+# undef ctime_r
char *asctime_r(struct tm const *restrict, char *restrict);
char *ctime_r(time_t const *, char *);
#endif /* HAVE_INCOMPATIBLE_CTIME_R */
diff --git a/tzfile.h b/tzfile.h
index f8c6c8c5..b00eb9e6 100644
--- a/tzfile.h
+++ b/tzfile.h
@@ -32,7 +32,7 @@
** Each file begins with. . .
*/
-#define TZ_MAGIC "TZif"
+#define TZ_MAGIC "TZif"
struct tzhead {
char tzh_magic[4]; /* TZ_MAGIC */
diff --git a/zic.c b/zic.c
index ef96efe7..854e306b 100644
--- a/zic.c
+++ b/zic.c
@@ -167,8 +167,8 @@ symlink(char const *target, char const *linkname)
}
#endif
#ifndef AT_SYMLINK_FOLLOW
-# define linkat(targetdir, target, linknamedir, linkname, flag) \
- (errno = ENOTSUP, -1)
+# define linkat(targetdir, target, linknamedir, linkname, flag) \
+ (errno = ENOTSUP, -1)
#endif
static void addtt(zic_t starttime, int type);
--
2.48.1
From c87f0918b01f1c40c0da1fc82bebc0252b932b9d Mon Sep 17 00:00:00 2001
From: Paul Eggert <[email protected]>
Date: Wed, 24 Sep 2025 15:37:08 -0700
Subject: [PROPOSED 2/4] Use strnlen
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
* Makefile, NEWS: Mention this.
* localtime.c (scrub_abbrs, tzloadbody, tzset_unlocked):
* zic.c (newabbr): Prefer strnlen to strlen if we merely want to
check whether a string fits.
* private.h (strnlen): Define if platform doesn’t.
---
Makefile | 1 +
NEWS | 3 +++
localtime.c | 9 +++++----
private.h | 12 ++++++++++++
zic.c | 2 +-
5 files changed, 22 insertions(+), 5 deletions(-)
diff --git a/Makefile b/Makefile
index 6aac7b2d..321b447d 100644
--- a/Makefile
+++ b/Makefile
@@ -267,6 +267,7 @@ LDLIBS=
# -DHAVE_STDINT_H=0 if <stdint.h> does not work*+
# -DHAVE_STRFTIME_L if <time.h> declares locale_t and strftime_l
# -DHAVE_STRDUP=0 if your system lacks the strdup function
+# -DHAVE_STRNLEN=0 if your system lacks the strnlen function+
# -DHAVE_STRTOLL=0 if your system lacks the strtoll function+
# -DHAVE_SYMLINK=0 if your system lacks the symlink function
# -DHAVE_SYS_STAT_H=0 if <sys/stat.h> does not work*
diff --git a/NEWS b/NEWS
index 2421950e..4f2d08e6 100644
--- a/NEWS
+++ b/NEWS
@@ -32,6 +32,9 @@ Unreleased, experimental changes
It is defined if STD_INSPIRED is defined.
(Patch from Dag-Erling Smørgrav.)
+ tzcode now uses strnlen to improve asymptotic performance a bit.
+ Compile with -DHAVE_STRNLEN=0 if your platform lacks it.
+
Changes to commentary
The leapseconds file contains commentary about the IERS and NIST
diff --git a/localtime.c b/localtime.c
index 378694fb..bae8217f 100644
--- a/localtime.c
+++ b/localtime.c
@@ -460,7 +460,7 @@ scrub_abbrs(struct state *sp)
/* Reject overlong abbreviations. */
for (i = 0; i < sp->charcnt - (TZNAME_MAXIMUM + 1); ) {
- int len = strlen(&sp->chars[i]);
+ int len = strnlen(&sp->chars[i], TZNAME_MAXIMUM + 1);
if (TZNAME_MAXIMUM < len)
return EOVERFLOW;
i += len + 1;
@@ -549,7 +549,8 @@ tzloadbody(char const *name, struct state *sp, char tzloadflags,
#endif
if (!doaccess) {
char const *dot;
- if (sizeof lsp->fullname - sizeof tzdirslash <= strlen(name))
+ if (sizeof lsp->fullname - sizeof tzdirslash
+ <= strnlen(name, sizeof lsp->fullname - sizeof tzdirslash))
return ENAMETOOLONG;
/* Create a string "TZDIR/NAME". Using sprintf here
@@ -793,7 +794,7 @@ tzloadbody(char const *name, struct state *sp, char tzloadflags,
break;
}
if (! (j < charcnt)) {
- int tsabbrlen = strlen(tsabbr);
+ int tsabbrlen = strnlen(tsabbr, TZ_MAX_CHARS - j);
if (j + tsabbrlen < TZ_MAX_CHARS) {
strcpy(sp->chars + j, tsabbr);
charcnt = j + tsabbrlen + 1;
@@ -1474,7 +1475,7 @@ tzset_unlocked(void)
{
char const *name = getenv("TZ");
struct state *sp = lclptr;
- int lcl = name ? strlen(name) < sizeof lcl_TZname : -1;
+ int lcl = name ? strnlen(name, sizeof lcl_TZname) < sizeof lcl_TZname : -1;
if (lcl < 0
? lcl_is_set < 0
: 0 < lcl_is_set && strcmp(lcl_TZname, name) == 0)
diff --git a/private.h b/private.h
index b2856e2e..223caf6b 100644
--- a/private.h
+++ b/private.h
@@ -212,7 +212,19 @@
#undef tzfree
#include <stddef.h>
+
#include <string.h>
+#if defined HAVE_STRNLEN && !HAVE_STRNLEN
+static size_t
+strnlen (char const *s, size_t maxlen)
+{
+ size_t i;
+ for (i = 0; i < maxlen && s[i]; i++)
+ continue;
+ return i;
+}
+#endif
+
#if !PORT_TO_C89
# include <inttypes.h>
#endif
diff --git a/zic.c b/zic.c
index 854e306b..6bdb039d 100644
--- a/zic.c
+++ b/zic.c
@@ -3932,7 +3932,7 @@ mp = _("time zone abbreviation differs from POSIX standard");
if (mp != NULL)
warning("%s (%s)", mp, string);
}
- i = strlen(string) + 1;
+ i = strnlen(string, TZ_MAX_CHARS - charcnt) + 1;
if (charcnt + i > TZ_MAX_CHARS) {
error(_("too many, or too long, time zone abbreviations"));
exit(EXIT_FAILURE);
--
2.48.1
From 2e1cfd4a8f0b40186563eda2f15c1dc6e1ffa8c3 Mon Sep 17 00:00:00 2001
From: Paul Eggert <[email protected]>
Date: Wed, 24 Sep 2025 23:05:51 -0700
Subject: [PROPOSED 3/4] Abbreviate a TZ starting with TZDIR/
This should help future optimizations and security brushups.
The idea is taken from FreeBSD localtime.c.
* localtime.c (TZLOAD_TZDIR_SUB): New flag.
(zoneinit): Implement it.
(tzset_unlocked): Use it. Abbreviate a longer string if possible.
---
localtime.c | 40 +++++++++++++++++++++++++++++++---------
1 file changed, 31 insertions(+), 9 deletions(-)
diff --git a/localtime.c b/localtime.c
index bae8217f..39501a90 100644
--- a/localtime.c
+++ b/localtime.c
@@ -513,6 +513,7 @@ union local_storage {
/* These tzload flags can be ORed together, and fit into 'char'. */
enum { TZLOAD_FROMENV = 1 }; /* The TZ string came from the environment. */
enum { TZLOAD_TZSTRING = 2 }; /* Read any newline-surrounded TZ string. */
+enum { TZLOAD_TZDIR_SUB = 4 }; /* TZ should be a file under TZDIR. */
/* Load tz data from the file named NAME into *SP. Respect TZLOADFLAGS.
Use *LSP for temporary storage. Return 0 on
@@ -1462,7 +1463,8 @@ zoneinit(struct state *sp, char const *name, char tzloadflags)
return 0;
} else {
int err = tzload(name, sp, tzloadflags);
- if (err != 0 && name && name[0] != ':' && tzparse(name, sp, NULL))
+ if (err != 0 && name && name[0] != ':' && !(tzloadflags & TZLOAD_TZDIR_SUB)
+ && tzparse(name, sp, NULL))
err = 0;
if (err == 0)
err = scrub_abbrs(sp);
@@ -1475,17 +1477,37 @@ tzset_unlocked(void)
{
char const *name = getenv("TZ");
struct state *sp = lclptr;
- int lcl = name ? strnlen(name, sizeof lcl_TZname) < sizeof lcl_TZname : -1;
- if (lcl < 0
- ? lcl_is_set < 0
- : 0 < lcl_is_set && strcmp(lcl_TZname, name) == 0)
+ char tzloadflags = TZLOAD_FROMENV | TZLOAD_TZSTRING;
+ size_t namelen = sizeof lcl_TZname + 1; /* placeholder for no name */
+
+ if (name) {
+ namelen = strnlen(name, sizeof lcl_TZname);
+
+ /* Abbreviate a string like "/usr/share/zoneinfo/America/Los_Angeles"
+ to its shorter equivalent "America/Los_Angeles". */
+ if (sizeof tzdirslash < namelen
+ && memcmp(name, tzdirslash, sizeof tzdirslash) == 0) {
+ char const *p = name + sizeof tzdirslash;
+ while (*p == '/')
+ p++;
+ if (*p && *p != ':') {
+ name = p;
+ namelen = strnlen(name, sizeof lcl_TZname);
+ tzloadflags |= TZLOAD_TZDIR_SUB;
+ }
+ }
+ }
+
+ if (name
+ ? 0 < lcl_is_set && strcmp(lcl_TZname, name) == 0
+ : lcl_is_set < 0)
return;
# ifdef ALL_STATE
if (! sp)
lclptr = sp = malloc(sizeof *lclptr);
# endif
if (sp) {
- int err = zoneinit(sp, name, TZLOAD_FROMENV | TZLOAD_TZSTRING);
+ int err = zoneinit(sp, name, tzloadflags);
if (err != 0) {
zoneinit(sp, "", 0);
/* Abbreviate with "-00" if there was an error.
@@ -1493,11 +1515,11 @@ tzset_unlocked(void)
if (name || err != ENOENT)
strcpy(sp->chars, UNSPEC);
}
- if (0 < lcl)
- strcpy(lcl_TZname, name);
+ if (namelen < sizeof lcl_TZname)
+ memcpy(lcl_TZname, name, namelen + 1);
}
settzname();
- lcl_is_set = lcl;
+ lcl_is_set = (sizeof lcl_TZname > namelen) - (sizeof lcl_TZname < namelen);
}
#endif
--
2.48.1
From 87abb1135ef7bd5d2e57041868ad9a135b9fa67d Mon Sep 17 00:00:00 2001
From: Paul Eggert <[email protected]>
Date: Fri, 26 Sep 2025 00:20:39 -0700
Subject: [PROPOSED 4/4] Tighten security checks on TZ values
These changes are inspired by checks made in FreeBSD localtime.
Also, do not check TZ strings differently if they come from the
environment, as that was confusing and undocumented.
* Makefile, NEWS, newtzset.3: Mention this.
* localtime.c [HAVE_GETAUXVAL&&!HAVE_ISSETUGID]: Include <sys/auxv.h>.
(issetugid) [!HAVE_ISSETUGID]: New static function.
(TZLOAD_FROMENV): Remove. All uses removed.
(tzloadbody): Implement these changes.
* private.h (HAVE_GETEUID, HAVE_GETRESUID, HAVE_GETAUXVAL)
(HAVE_ISSETUGID): Provide reasonable defaults in the typical
case where CFLAGS does not define these.
(ENOTCAPABLE): Default to EINVAL.
(R_OK): Remove; no longer used.
---
Makefile | 4 ++
NEWS | 19 +++++++++
localtime.c | 118 ++++++++++++++++++++++++++++++++++++----------------
newtzset.3 | 12 +++++-
private.h | 40 +++++++++++++++---
5 files changed, 150 insertions(+), 43 deletions(-)
diff --git a/Makefile b/Makefile
index 321b447d..83122ecc 100644
--- a/Makefile
+++ b/Makefile
@@ -244,8 +244,11 @@ LDLIBS=
# -DHAVE_DECL_TIMEGM=0 if <time.h> does not declare timegm
# -DHAVE_DIRECT_H if mkdir needs <direct.h> (MS-Windows)
# -DHAVE__GENERIC=0 if _Generic does not work*
+# -DHAVE_GETAUXVAL=1 if getauxval works, 0 otherwise (default is guessed)
+# -DHAVE_GETEUID=0 if gete?[ug]id do not work
# -DHAVE_GETRANDOM if getrandom works (e.g., GNU/Linux),
# -DHAVE_GETRANDOM=0 to avoid using getrandom
+# -DHAVE_GETRESUID=0 if getres[ug]id do not work
# -DHAVE_GETTEXT if gettext works (e.g., GNU/Linux, FreeBSD, Solaris),
# where LDLIBS also needs to contain -lintl on some hosts;
# -DHAVE_GETTEXT=0 to avoid using gettext
@@ -253,6 +256,7 @@ LDLIBS=
# ctime_r and asctime_r incompatibly with POSIX.1-2017 and earlier
# (Solaris when _POSIX_PTHREAD_SEMANTICS is not defined).
# -DHAVE_INTTYPES_H=0 if <inttypes.h> does not work*+
+# -DHAVE_ISSETUID=1 if issetugid works, 0 otherwise (default is guessed)
# -DHAVE_LINK=0 if your system lacks a link function
# -DHAVE_LOCALTIME_R=0 if your system lacks a localtime_r function
# -DHAVE_LOCALTIME_RZ=0 if you do not want zdump to use localtime_rz
diff --git a/NEWS b/NEWS
index 4f2d08e6..66912d2e 100644
--- a/NEWS
+++ b/NEWS
@@ -32,6 +32,25 @@ Unreleased, experimental changes
It is defined if STD_INSPIRED is defined.
(Patch from Dag-Erling Smørgrav.)
+ tzcode is now more cautious about questionable TZ settings.
+ Privileged programs now reject TZ settings that start with '/',
+ unless they are TZDEFAULT (default "/etc/localtime") or
+ start with TZDIR then "/" (default "/usr/share/zoneinfo/").
+ Unprivileged programs now require files to be regular files
+ and reject relative names containing ".." directory components;
+ formerly, only privileged programs did those two things.
+ These changes were inspired by similar behavior in FreeBSD.
+ The code no longer treat TZ strings taken from tzalloc arguments
+ with less caution than TZ strings taken from the environment, as
+ this was undocumented and would have been hard to explain anyway.
+ tzcode no longer uses the 'access' system call to check access;
+ Instead it now uses the system calls issetuid, getauxval,
+ getresuid/getresgid, and geteuid/getegid/getuid/getgid (whichever
+ first works) to test whether a program is privileged;
+ compile with -DHAVE_ISSETUID=[01], -DHAVE_GETAUXVAL=[01],
+ -DHAVE_GETRESUID=[01], and -DHAVE_GETEUID=[01] to enable or
+ disable these system calls' use.
+
tzcode now uses strnlen to improve asymptotic performance a bit.
Compile with -DHAVE_STRNLEN=0 if your platform lacks it.
diff --git a/localtime.c b/localtime.c
index 39501a90..32013247 100644
--- a/localtime.c
+++ b/localtime.c
@@ -19,6 +19,10 @@
#include "tzfile.h"
#include <fcntl.h>
+#if HAVE_GETAUXVAL && !HAVE_ISSETUGID
+# include <sys/auxv.h>
+#endif
+
#if HAVE_SYS_STAT_H
# include <sys/stat.h>
#endif
@@ -132,6 +136,38 @@ typedef intmax_t timex_t;
# define O_NOCTTY 0
#endif
+/* Return 1 if the process is privileged, 0 otherwise. */
+#if !HAVE_ISSETUGID
+static int
+issetugid(void)
+{
+# if HAVE_GETAUXVAL && defined AT_SECURE
+ unsigned long val;
+ errno = 0;
+ val = getauxval(AT_SECURE);
+ if (val || errno != ENOENT)
+ return !!val;
+# endif
+# if HAVE_GETRESUID
+ {
+ uid_t ruid, euid, suid;
+ gid_t rgid, egid, sgid;
+ if (0 <= getresuid (&ruid, &euid, &suid)) {
+ if ((ruid ^ euid) | (ruid ^ suid))
+ return 1;
+ if (0 <= getresgid (&rgid, &egid, &sgid))
+ return !!((rgid ^ egid) | (rgid ^ sgid));
+ }
+ }
+# endif
+# if HAVE_GETEUID
+ return geteuid() != getuid() || getegid() != getgid();
+# else
+ return 0;
+# endif
+}
+#endif
+
#ifndef WILDABBR
/*
** Someone might make incorrect use of a time zone abbreviation:
@@ -511,9 +547,8 @@ union local_storage {
};
/* These tzload flags can be ORed together, and fit into 'char'. */
-enum { TZLOAD_FROMENV = 1 }; /* The TZ string came from the environment. */
+enum { TZLOAD_TZDIR_SUB = 1 }; /* TZ should be a file under TZDIR. */
enum { TZLOAD_TZSTRING = 2 }; /* Read any newline-surrounded TZ string. */
-enum { TZLOAD_TZDIR_SUB = 4 }; /* TZ should be a file under TZDIR. */
/* Load tz data from the file named NAME into *SP. Respect TZLOADFLAGS.
Use *LSP for temporary storage. Return 0 on
@@ -526,7 +561,7 @@ tzloadbody(char const *name, struct state *sp, char tzloadflags,
register int fid;
register int stored;
register ssize_t nread;
- register bool doaccess;
+ char const *relname;
register union input_buffer *up = &lsp->u.u;
register int tzheadsize = sizeof(struct tzhead);
@@ -536,20 +571,55 @@ tzloadbody(char const *name, struct state *sp, char tzloadflags,
name = TZDEFAULT;
if (! name)
return EINVAL;
- tzloadflags &= ~TZLOAD_FROMENV;
}
if (name[0] == ':')
++name;
+
+ relname = name;
+
+ /* If the program is privileged, NAME is TZDEFAULT or
+ subsidiary to TZDIR. Also, NAME is not a device. */
+ if (name[0] == '/' && strcmp(name, TZDEFAULT) != 0) {
+ if (strncmp(relname, tzdirslash, sizeof tzdirslash) == 0)
+ for (relname += sizeof tzdirslash; *relname == '/'; relname++)
+ continue;
+ else if (issetugid())
+ return ENOTCAPABLE;
+ else {
+#ifdef S_ISREG
+ /* Check for devices, as their mere opening could have
+ unwanted side effects. Though racy, there is no
+ portable way to fix the races. This check is needed
+ only for files not otherwise known to be non-devices. */
+ struct stat st;
+ if (stat(name, &st) < 0)
+ return errno;
+ if (!S_ISREG(st.st_mode))
+ return EINVAL;
+#endif
+ }
+ }
+
+ /* Fail if a relative name contains a ".." component,
+ as such a name could read a file outside TZDIR. */
+ if (relname[0] != '/') {
+ char const *component;
+ for (component = relname; ; component++) {
+ if (component[0] == '.' && component[1] == '.'
+ && ((component[2] == '/') | !component[2]))
+ return ENOTCAPABLE;
+ component = strchr(component, '/');
+ if (!component)
+ break;
+ }
+ }
+
#ifdef SUPPRESS_TZDIR
/* Do not prepend TZDIR. This is intended for specialized
applications only, due to its security implications. */
- doaccess = true;
#else
- doaccess = name[0] == '/';
-#endif
- if (!doaccess) {
- char const *dot;
+ if (name[0] != '/') {
if (sizeof lsp->fullname - sizeof tzdirslash
<= strnlen(name, sizeof lsp->fullname - sizeof tzdirslash))
return ENAMETOOLONG;
@@ -559,36 +629,10 @@ tzloadbody(char const *name, struct state *sp, char tzloadflags,
resulting string length exceeded INT_MAX!). */
memcpy(lsp->fullname, tzdirslash, sizeof tzdirslash);
strcpy(lsp->fullname + sizeof tzdirslash, name);
-
- /* Set doaccess if NAME contains a ".." file name
- component, as such a name could read a file outside
- the TZDIR virtual subtree. */
- for (dot = name; (dot = strchr(dot, '.')); dot++)
- if ((dot == name || dot[-1] == '/') && dot[1] == '.'
- && (dot[2] == '/' || !dot[2])) {
- doaccess = true;
- break;
- }
-
name = lsp->fullname;
}
- if (doaccess && (tzloadflags & TZLOAD_FROMENV)) {
- /* Check for security violations and for devices whose mere
- opening could have unwanted side effects. Although these
- checks are racy, they're better than nothing and there is
- no portable way to fix the races. */
- if (access(name, R_OK) < 0)
- return errno;
-#ifdef S_ISREG
- {
- struct stat st;
- if (stat(name, &st) < 0)
- return errno;
- if (!S_ISREG(st.st_mode))
- return EINVAL;
- }
#endif
- }
+
fid = open(name, (O_RDONLY | O_BINARY | O_CLOEXEC | O_CLOFORK
| O_IGNORE_CTTY | O_NOCTTY));
if (fid < 0)
@@ -1477,7 +1521,7 @@ tzset_unlocked(void)
{
char const *name = getenv("TZ");
struct state *sp = lclptr;
- char tzloadflags = TZLOAD_FROMENV | TZLOAD_TZSTRING;
+ char tzloadflags = TZLOAD_TZSTRING;
size_t namelen = sizeof lcl_TZname + 1; /* placeholder for no name */
if (name) {
diff --git a/newtzset.3 b/newtzset.3
index 5358475d..3801ab1b 100644
--- a/newtzset.3
+++ b/newtzset.3
@@ -78,7 +78,17 @@ contents are used as a pathname, a pathname beginning with
is used as-is; otherwise
the pathname is relative to a system time conversion information
directory.
-The file must be in the format specified in
+In a privileged program the pathname must be relative.
+Relative pathnames must not contain
+.q "..\&"
+components.
+For the purpose of these checks, a file name beginning with
+.q "/"
+is considered to be relative if it is the
+.B localtime
+file's name, or if it starts with the
+system timezone directory's name followed by one more more slashes.
+The file must be a regular file in the format specified in
.BR tzfile (5).
.PP
When
diff --git a/private.h b/private.h
index 223caf6b..6ae0c2fd 100644
--- a/private.h
+++ b/private.h
@@ -118,6 +118,14 @@
# define HAVE__GENERIC (201112 <= __STDC_VERSION__)
#endif
+#ifndef HAVE_GETEUID
+# define HAVE_GETEUID 1
+#endif
+
+#ifndef HAVE_GETRESUID
+# define HAVE_GETRESUID 1
+#endif
+
#if !defined HAVE_GETTEXT && defined __has_include
# if __has_include(<libintl.h>)
# define HAVE_GETTEXT 1
@@ -246,6 +254,9 @@ strnlen (char const *s, size_t maxlen)
#ifndef ENOMEM
# define ENOMEM EINVAL
#endif
+#ifndef ENOTCAPABLE
+# define ENOTCAPABLE EINVAL
+#endif
#ifndef ENOTSUP
# define ENOTSUP EINVAL
#endif
@@ -258,7 +269,7 @@ strnlen (char const *s, size_t maxlen)
#endif /* HAVE_GETTEXT */
#if HAVE_UNISTD_H
-# include <unistd.h> /* for R_OK, and other POSIX goodness */
+# include <unistd.h>
#endif /* HAVE_UNISTD_H */
/* SUPPORT_POSIX2008 means the tzcode library should support
@@ -286,6 +297,29 @@ strnlen (char const *s, size_t maxlen)
# endif
#endif
+#if !defined HAVE_GETAUXVAL && defined __has_include
+# if __has_include(<sys/auxv.h>)
+# define HAVE_GETAUXVAL 1
+# endif
+#endif
+#ifndef HAVE_GETAUXVAL
+# if defined __GLIBC__ && 2 < __GLIBC__ + (19 <= __GLIBC_MINOR__)
+# define HAVE_GETAUXVAL 1
+# else
+# define HAVE_GETAUXVAL 0
+# endif
+#endif
+
+#ifndef HAVE_ISSETUGID
+# if (defined __FreeBSD__ || defined __NetBSD__ || defined __OpenBSD__ \
+ || (defined __linux__ && !defined __GLIBC__) /* Android, musl, etc. */ \
+ || (defined __APPLE__ && defined __MACH__) || defined __sun)
+# define HAVE_ISSETUGID 1
+# else
+# define HAVE_ISSETUGID 0
+# endif
+#endif
+
#ifndef HAVE_SNPRINTF
# define HAVE_SNPRINTF (!PORT_TO_C89 || 199901 <= __STDC_VERSION__)
#endif
@@ -322,10 +356,6 @@ strnlen (char const *s, size_t maxlen)
# endif
#endif
-#ifndef R_OK
-# define R_OK 4
-#endif /* !defined R_OK */
-
#if PORT_TO_C89
/*
--
2.48.1
