Follow @Openwall on Twitter for new release announcements and other news
[<prev] [<thread-prev] [day] [month] [year] [list]
Message-ID: <20260806000235.GW27423@brightrain.aerifal.cx>
Date: Wed, 5 Aug 2026 20:02:35 -0400
From: Rich Felker <dalias@...c.org>
To: Matthias Goergens <matthias.goergens@...il.com>
Cc: musl@...ts.openwall.com
Subject: Re: [PATCH] time: fix TZif version and footer parsing

On Wed, Aug 05, 2026 at 03:57:51PM +0800, Matthias Goergens wrote:
> TZif v1 encodes its version as NUL, but do_tzset treats only the byte
> '1' as v1. It consequently reads a valid v1 file as if it contained a
> second header.
> 
> Footer detection also scans backwards from any terminal newline. It can
> run before the mapping or mistake a newline in binary data for the
> footer delimiter.
> 
> Use zero and nonzero version bytes to distinguish v1 from later files.
> Compute the selected data block's end and accept a footer only there.
> ---
>  src/time/__tz.c | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/src/time/__tz.c b/src/time/__tz.c
> index cfce268e..69ef0712 100644
> --- a/src/time/__tz.c
> +++ b/src/time/__tz.c
> @@ -196,7 +196,7 @@ static void do_tzset()
>  	zi = map;
>  	if (map) {
>  		int scale = 2;
> -		if (map[4]!='1') {
> +		if (map[4]) {
>  			size_t skip = zi_dotprod(zi+20, VEC(1,1,8,5,6,1), 6);
>  			trans = zi+skip+44+44;
>  			scale++;
> @@ -207,9 +207,11 @@ static void do_tzset()
>  		types = index + zi_read32(trans-12);
>  		abbrevs = types + 6*zi_read32(trans-8);
>  		abbrevs_end = abbrevs + zi_read32(trans-4);
> -		if (zi[map_size-1] == '\n') {
> -			for (s = (const char *)zi+map_size-2; *s!='\n'; s--);
> -			s++;
> +		size_t footer = trans-zi + zi_dotprod(trans-24,
> +			VEC(1,1,(1<<scale)+4,(1<<scale)+1,6,1), 6);
> +		if (map[4] && footer < map_size-1 && zi[footer] == '\n'
> +		    && zi[map_size-1] == '\n') {
> +			s = (const char *)zi + footer + 1;
>  		} else {
>  			const unsigned char *p;
>  			__tzname[0] = __tzname[1] = 0;
> -- 
> 2.55.0

The v1 part should definitely be accepted; it's a bug.

But the second part gets into validating tzfiles, which is not
something we generally do. There are all sorts of ways a malformed tz
file can cause erroneous loads, and presently no guards against that,
since these files are considered trusted (and may not be loaded from
untrusted paths in a suid context). I'm not sure if we should get into
doing some level of validating tzfiles, but if so that's a direction
that should be agreed upon and have an overall goal and intent about
what we can and cannot support (made difficult by this being shared
memory that can change out from under us at any time), not just ad-hoc
additions mixed in with bug fixes.

If there is another reason we really should do the footer offset
calculation at this time (issue with newer file formats?) that I'm
missing, please let me know; in that case it should be ok, but still
separate from the bug fix.

Rich

Powered by blists - more mailing lists

Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.