Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Wed, 02 Jun 2021 17:41:30 +0200
From: Sören Tempel <soeren@...ren-tempel.net>
To: musl@...ts.openwall.com
Subject: Re: Handling of non-location specific TZ values

Ping.

Would be willing to adjust the patch as needed. In any case, it would be
nice to get this fixed as it currently causes some test failures of
Alpine packages.

Greetings,
Sören

Sören Tempel <soeren@...ren-tempel.net> wrote:
> Rich Felker <dalias@...c.org> wrote:
> > Yes. I suspect we can get by with calling getname with a dummy output
> > array, then checking if the next character is one of +, -, or a digit.
> > If not (in particular, if it's a null character) then we can attempt
> > loading it as a file.
> 
> Maybe something along the following? Not too familiar with the musl code
> base so not sure if including ctype.h is allowed etc.
> 
> diff --git a/src/time/__tz.c b/src/time/__tz.c
> index 09a6317e..6bc183d0 100644
> --- a/src/time/__tz.c
> +++ b/src/time/__tz.c
> @@ -3,6 +3,7 @@
>  #include <limits.h>
>  #include <stdlib.h>
>  #include <string.h>
> +#include <ctype.h>
>  #include <sys/mman.h>
>  #include "libc.h"
>  #include "lock.h"
> @@ -125,13 +126,13 @@ static size_t zi_dotprod(const unsigned char *z, const unsigned char *v, size_t
>  static void do_tzset()
>  {
>  	char buf[NAME_MAX+25], *pathname=buf+24;
> -	const char *try, *s, *p;
> +	const char *try, *s, *orig;
>  	const unsigned char *map = 0;
>  	size_t i;
>  	static const char search[] =
>  		"/usr/share/zoneinfo/\0/share/zoneinfo/\0/etc/zoneinfo/\0";
> 
> -	s = getenv("TZ");
> +	s = orig = getenv("TZ");
>  	if (!s) s = "/etc/localtime";
>  	if (!*s) s = __utc;
> 
> @@ -154,11 +155,19 @@ static void do_tzset()
>  	}
>  	if (old_tz) memcpy(old_tz, s, i+1);
> 
> -	/* Non-suid can use an absolute tzfile pathname or a relative
> -	 * pathame beginning with "."; in secure mode, only the
> -	 * standard path will be searched. */
> -	if (*s == ':' || ((p=strchr(s, '/')) && !memchr(s, ',', p-s))) {
> +	/* The TZ format specified by POSIX consists of a mandatory
> +	 * time zone name and a mandatory offset. We determine the
> +	 * name using getname, if the next character cannot constitute
> +	 * a valid offset (or the TZ value starts with a colon) we
> +	 * interpret the TZ environment variable as a zoneinfo file name. */
> +	getname(std_name, &s);
> +	if (*s == ':' || (!isdigit(*s) && *s != '+' && *s != '-')) {
>  		if (*s == ':') s++;
> +		else if (orig) s = orig;
> +
> +		/* Non-suid can use an absolute tzfile pathname or a relative
> +		 * pathame beginning with "."; in secure mode, only the
> +		 * standard path will be searched. */
>  		if (*s == '/' || *s == '.') {
>  			if (!libc.secure || !strcmp(s, "/etc/localtime"))
>  				map = __map_file(s, &map_size);
> 
> Patch can be tested using date(1). For instance, compare the output of
> `TZ=CET date` on a patched and unpatched system with an installed
> zoneinfo database.
> 
> > It might be worth adding a special exception for "UTC" and "GMT" so
> > that they are always interpreted as "UTC0" and "GMT0" and can't be
> > overridden by a bogus file in the zoneinfo path, for the sake of
> > software that does "TZ=UTC cmd" to avoid any timezone shenanigans.
> 
> Maybe that can be done in a separate commit?
> 
> Greetings,
> Sören

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.