On 2025-03-05 21:56, Евгений Горбанев wrote:
I found this scenario while researching the postgresql code using Svace
static analyzer.
Well, try this scenario:
$ cd /tmp
$ wget https://data.iana.org/time-zones/releases/tzdb-2025a.tar.lz
$ tar xf tzdb-2025a.tar.lz
$ cd tzdb-2025a
$ make CFLAGS='-fsanitize=address -g'
$ touch test
$ ./zic -l test -d .
Thanks for the explanation and recipe. I can reproduce the problem.
Please try the attached proposed patch. I installed it into the
development repository <https://github.com/eggert/tz>. Thanks.From 7063d08cdde2ce1afa2179195d4340f0118461b8 Mon Sep 17 00:00:00 2001
From: Paul Eggert <egg...@cs.ucla.edu>
Date: Thu, 6 Mar 2025 00:03:53 -0800
Subject: [PROPOSED] Fix bug with -d RELATIVE -t ABSOLUTE
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Problem reported by Evgeniy Gorbanev in:
https://lists.iana.org/hyperkitty/list/tz@iana.org/thread/Y4IQO6VNGAALRTS2TEACPHZBUMTIPHEZ/
Fix the main bug here by not trying to create a symlink
with contents that won’t work because they’re relative.
This also fixes the read buffer underflow.
While we’re at it, fix some bogus diagnostics issued when
a file name is absolute.
* NEWS: Mention this.
* zic.c (close_file): If NAME is absolute, don’t output any
directory that it’s relative to.
(directory_end_in_slash): New static var.
(main): Set it after changing directory, in case directory is empty.
(diagdir, diagslash): New functions.
(open_outfile, rename_dest, dolink): Use them in diagnostics to
handle the unusual case where an absolute file name is used.
(relname): Return a null pointer if LINKNAME is absolute but
DIRECTORY is not, because the computed symlink wouldn’t work
in that case.
(dolink): If relname returns a null pointer, don’t try
to create a symlink; just make a copy. When warning about the
copy, say that the symlink is not obvious.
---
NEWS | 8 ++++++
zic.c | 79 +++++++++++++++++++++++++++++++++++++++++------------------
2 files changed, 63 insertions(+), 24 deletions(-)
diff --git a/NEWS b/NEWS
index bc146e89..eec41543 100644
--- a/NEWS
+++ b/NEWS
@@ -1,5 +1,13 @@
News for the tz database
+Unreleased, experimental changes
+
+ Changes to code
+
+ 'zic -l TIMEZONE -d . -l /some/other/file/system' no longer has a
+ read buffer underflow. (Problem reported by Evgeniy Gorbanev.)
+
+
Release 2025a - 2025-01-15 10:47:24 -0800
Briefly:
diff --git a/zic.c b/zic.c
index 8a7f83db..794927ea 100644
--- a/zic.c
+++ b/zic.c
@@ -652,6 +652,8 @@ close_file(FILE *stream, char const *dir, char const *name,
char const *e = (ferror(stream) ? _("I/O error")
: fclose(stream) != 0 ? strerror(errno) : NULL);
if (e) {
+ if (name && *name == '/')
+ dir = NULL;
fprintf(stderr, "%s: %s%s%s%s%s\n", progname,
dir ? dir : "", dir ? "/" : "",
name ? name : "", name ? ": " : "",
@@ -953,6 +955,9 @@ static const char * lcltime;
static const char * directory;
static const char * tzdefault;
+/* True if DIRECTORY ends in '/'. */
+static bool directory_ends_in_slash;
+
/* -1 if the TZif output file should be slim, 0 if default, 1 if the
output should be fat for backward compatibility. ZIC_BLOAT_DEFAULT
determines the default. */
@@ -1137,6 +1142,7 @@ main(int argc, char **argv)
return EXIT_FAILURE;
associate();
change_directory(directory);
+ directory_ends_in_slash = directory[strlen(directory) - 1] == '/';
catch_signals();
for (i = 0; i < nzones; i = j) {
/*
@@ -1338,6 +1344,20 @@ random_dirent(char const **name, char **namealloc)
}
}
+/* For diagnostics the directory, and file name relative to that
+ directory, respectively. A diagnostic routne can name FILENAME by
+ outputting diagdir(FILENAME), then diagslash(FILENAME), then FILENAME. */
+static char const *
+diagdir(char const *filename)
+{
+ return *filename == '/' ? "" : directory;
+}
+static char const *
+diagslash(char const *filename)
+{
+ return &"/"[*filename == '/' || directory_ends_in_slash];
+}
+
/* Prepare to write to the file *OUTNAME, using *TEMPNAME to store the
name of the temporary file that will eventually be renamed to
*OUTNAME. Assign the temporary file's name to both *OUTNAME and
@@ -1366,8 +1386,9 @@ open_outfile(char const **outname, char **tempname)
} else if (fopen_errno == EEXIST)
random_dirent(outname, tempname);
else {
- fprintf(stderr, _("%s: Can't create %s/%s: %s\n"),
- progname, directory, *outname, strerror(fopen_errno));
+ fprintf(stderr, _("%s: Can't create %s%s%s: %s\n"),
+ progname, diagdir(*outname), diagslash(*outname), *outname,
+ strerror(fopen_errno));
exit(EXIT_FAILURE);
}
}
@@ -1385,8 +1406,9 @@ rename_dest(char *tempname, char const *name)
if (rename(tempname, name) != 0) {
int rename_errno = errno;
remove(tempname);
- fprintf(stderr, _("%s: rename to %s/%s: %s\n"),
- progname, directory, name, strerror(rename_errno));
+ fprintf(stderr, _("%s: rename to %s%s%s: %s\n"),
+ progname, diagdir(name), diagslash(name), name,
+ strerror(rename_errno));
exit(EXIT_FAILURE);
}
free(tempname);
@@ -1396,7 +1418,8 @@ rename_dest(char *tempname, char const *name)
/* Create symlink contents suitable for symlinking TARGET to LINKNAME, as a
freshly allocated string. TARGET should be a relative file name, and
is relative to the global variable DIRECTORY. LINKNAME can be either
- relative or absolute. */
+ relative or absolute. Return a null pointer if the symlink contents
+ was not computed because LINKNAME is absolute but DIRECTORY is not. */
static char *
relname(char const *target, char const *linkname)
{
@@ -1409,6 +1432,8 @@ relname(char const *target, char const *linkname)
size_t len = strlen(directory);
size_t lenslash = len + (len && directory[len - 1] != '/');
size_t targetsize = strlen(target) + 1;
+ if (*directory != '/')
+ return NULL;
linksize = size_sum(lenslash, targetsize);
f = result = xmalloc(linksize);
memcpy(result, directory, len);
@@ -1460,8 +1485,9 @@ dolink(char const *target, char const *linkname, bool staysymlink)
return;
else {
char const *e = strerror(errno);
- fprintf(stderr, _("%s: Can't remove %s/%s: %s\n"),
- progname, directory, linkname, e);
+ fprintf(stderr, _("%s: Can't remove %s%s%s: %s\n"),
+ progname, diagdir(linkname), diagslash(linkname), linkname,
+ e);
exit(EXIT_FAILURE);
}
}
@@ -1504,8 +1530,9 @@ dolink(char const *target, char const *linkname, bool staysymlink)
mkdirs(linkname, true);
linkdirs_made = true;
} else {
- fprintf(stderr, _("%s: Can't link %s/%s to %s/%s: %s\n"),
- progname, directory, target, directory, outname,
+ fprintf(stderr, _("%s: Can't link %s%s%s to %s%s%s: %s\n"),
+ progname, diagdir(target), diagslash(target), target,
+ diagdir(outname), diagslash(outname), outname,
strerror(link_errno));
exit(EXIT_FAILURE);
}
@@ -1514,21 +1541,23 @@ dolink(char const *target, char const *linkname, bool staysymlink)
bool absolute = *target == '/';
char *linkalloc = absolute ? NULL : relname(target, linkname);
char const *contents = absolute ? target : linkalloc;
- int symlink_errno;
+ int symlink_errno = -1;
- while (true) {
- if (symlink(contents, outname) == 0) {
- symlink_errno = 0;
- break;
+ if (contents) {
+ while (true) {
+ if (symlink(contents, outname) == 0) {
+ symlink_errno = 0;
+ break;
+ }
+ symlink_errno = errno;
+ if (symlink_errno == EEXIST)
+ random_dirent(&outname, &tempname);
+ else if (symlink_errno == ENOENT && !linkdirs_made) {
+ mkdirs(linkname, true);
+ linkdirs_made = true;
+ } else
+ break;
}
- symlink_errno = errno;
- if (symlink_errno == EEXIST)
- random_dirent(&outname, &tempname);
- else if (symlink_errno == ENOENT && !linkdirs_made) {
- mkdirs(linkname, true);
- linkdirs_made = true;
- } else
- break;
}
free(linkalloc);
if (symlink_errno == 0) {
@@ -1541,8 +1570,8 @@ dolink(char const *target, char const *linkname, bool staysymlink)
fp = fopen(target, "rb");
if (!fp) {
char const *e = strerror(errno);
- fprintf(stderr, _("%s: Can't read %s/%s: %s\n"),
- progname, directory, target, e);
+ fprintf(stderr, _("%s: Can't read %s%s%s: %s\n"),
+ progname, diagdir(target), diagslash(target), target, e);
exit(EXIT_FAILURE);
}
tp = open_outfile(&outname, &tempname);
@@ -1553,6 +1582,8 @@ dolink(char const *target, char const *linkname, bool staysymlink)
if (link_errno != ENOTSUP)
warning(_("copy used because hard link failed: %s"),
strerror(link_errno));
+ else if (symlink_errno < 0)
+ warning(_("copy used because symbolic link not obvious"));
else if (symlink_errno != ENOTSUP)
warning(_("copy used because symbolic link failed: %s"),
strerror(symlink_errno));
--
2.45.2
