diff mbox series

[v2] time: Avoid memcmp overread in tzset (bug 31931)

Message ID 878qyrvc5h.fsf@oldenburg.str.redhat.com
State New
Headers show
Series [v2] time: Avoid memcmp overread in tzset (bug 31931) | expand

Commit Message

Florian Weimer June 26, 2024, 5:36 p.m. UTC 
The test does not necessarily trigger the crash, depending on memcmp
behavior.  A crash was observed in __memcmp_ia32 on i686 builds.

Tested on i686-linux-gnu.

---
v2: Style fix for boolean comparison.  Fix putenv argument in the test.
 time/Makefile           |  5 ++++-
 time/tst-tzfile-fault.c | 44 ++++++++++++++++++++++++++++++++++++++++++++
 time/tzfile.c           |  5 +++--
 3 files changed, 51 insertions(+), 3 deletions(-)


base-commit: a10b6ad471d7b528149f5ff32eef2f1c1dc1213c

Comments

Paul Eggert June 26, 2024, 10:35 p.m. UTC | #1 
Thanks, this looks good.

Reviewed-by: Paul Eggert <eggert@cs.ucla.edu>
diff mbox series

Patch

diff --git a/time/Makefile b/time/Makefile
index 5b541fb9d3..f4c75b786d 100644
--- a/time/Makefile
+++ b/time/Makefile
@@ -50,7 +50,8 @@  tests	:= test_time clocktest tst-posixtz tst-strptime tst_wcsftime \
 	   tst-clock tst-clock2 tst-clock_nanosleep tst-cpuclock1 \
 	   tst-adjtime tst-ctime tst-difftime tst-mktime4 tst-clock_settime \
 	   tst-settimeofday tst-itimer tst-gmtime tst-timegm \
-	   tst-timespec_get tst-timespec_getres tst-strftime4
+	   tst-timespec_get tst-timespec_getres tst-strftime4 \
+	   tst-tzfile-fault
 
 tests-time64 := \
   tst-adjtime-time64 \
@@ -110,3 +111,5 @@  tst-tzname-ENV = TZDIR=${common-objpfx}timezone/testdata
 CPPFLAGS-tst-tzname.c += -DTZDEFRULES='"$(posixrules-file)"'
 
 bug-getdate1-ARGS = ${objpfx}bug-getdate1-fmt
+
+tst-tzfile-fault-ENV = GLIBC_TUNABLES=glibc.rtld.enable_secure=1
diff --git a/time/tst-tzfile-fault.c b/time/tst-tzfile-fault.c
new file mode 100644
index 0000000000..105c2ac7ce
--- /dev/null
+++ b/time/tst-tzfile-fault.c
@@ -0,0 +1,44 @@ 
+/* Attempt to trigger fault with very short TZ variable (bug 31931).
+   Copyright (C) 2024 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <https://www.gnu.org/licenses/>.  */
+
+
+#include <support/next_to_fault.h>
+#include <stdlib.h>
+#include <time.h>
+#include <string.h>
+
+static char tz[] = "TZ=/";
+
+static int
+do_test (void)
+{
+  struct support_next_to_fault ntf
+    = support_next_to_fault_allocate (sizeof (tz));
+  memcpy (ntf.buffer, tz, sizeof (tz));
+  putenv (ntf.buffer);
+
+  tzset ();
+
+  /* Avoid dangling pointer in environ.  */
+  putenv (tz);
+  support_next_to_fault_free (&ntf);
+
+  return 0;
+}
+
+#include <support/test-driver.c>
diff --git a/time/tzfile.c b/time/tzfile.c
index 4147539964..964a9b7f12 100644
--- a/time/tzfile.c
+++ b/time/tzfile.c
@@ -134,8 +134,9 @@  __tzfile_read (const char *file, size_t extra, char **extrap)
 	 and which is not the system wide default TZDEFAULT.  */
       if (__libc_enable_secure
 	  && ((*file == '/'
-	       && memcmp (file, TZDEFAULT, sizeof TZDEFAULT)
-	       && memcmp (file, default_tzdir, sizeof (default_tzdir) - 1))
+	       && strcmp (file, TZDEFAULT) != 0
+	       && (strncmp (file, default_tzdir, sizeof (default_tzdir) - 1)
+		   != 0))
 	      || strstr (file, "../") != NULL))
 	/* This test is certainly a bit too restrictive but it should
 	   catch all critical cases.  */