diff mbox

Fix BZ 18985 out of bounds access in strftime

Message ID CALoOobP2BQhWp_2NAvFyXfo9UVwr8mQCKbyL-8SJF8U3VoQRsA@mail.gmail.com
State New
Headers show

Commit Message

Paul Pluzhnikov Sept. 20, 2015, 8:15 p.m. UTC 
On Sun, Sep 20, 2015 at 11:45 AM, Paul Pluzhnikov
<ppluzhnikov@google.com> wrote:
> On Sun, Sep 20, 2015 at 11:39 AM, Paul Eggert <eggert@cs.ucla.edu> wrote:

>> Don't forget out-of-range tm_isdst: there's one place the code has undefined
>> behavior if tm_isdst exceeds 1.

Let's try again, now with a test case :-)


2015-09-20  Paul Pluzhnikov  <ppluzhnikov@google.com>

        [BZ #18985]
        * time/strftime_l.c (a_wkday, f_wkday, a_month, f_month): Range check.
        (__strftime_internal): Likewise.
        * time/tst-strftime.c (do_bz18985): New test.
        (do_test): Call it.

Comments

Andreas Schwab Sept. 20, 2015, 9:54 p.m. UTC | #1 
Paul Pluzhnikov <ppluzhnikov@google.com> writes:

> diff --git a/time/strftime_l.c b/time/strftime_l.c
> index b48ef34..619e4e3 100644
> --- a/time/strftime_l.c
> +++ b/time/strftime_l.c
> @@ -510,13 +510,17 @@ __strftime_internal (s, maxsize, format, tp, tzset_called ut_argument
>       only a few elements.  Dereference the pointers only if the format
>       requires this.  Then it is ok to fail if the pointers are invalid.  */
>  # define a_wkday \
> -  ((const CHAR_T *) _NL_CURRENT (LC_TIME, NLW(ABDAY_1) + tp->tm_wday))
> +  ((const CHAR_T *) (tp->tm_wday < 0 || tp->tm_wday > 6 ?		      \
> +		     "?" : _NL_CURRENT (LC_TIME, NLW(ABDAY_1) + tp->tm_wday)))
>  # define f_wkday \
> -  ((const CHAR_T *) _NL_CURRENT (LC_TIME, NLW(DAY_1) + tp->tm_wday))
> +  ((const CHAR_T *) (tp->tm_wday < 0 || tp->tm_wday > 6 ?		      \
> +		     "?" : _NL_CURRENT (LC_TIME, NLW(DAY_1) + tp->tm_wday)))
>  # define a_month \
> -  ((const CHAR_T *) _NL_CURRENT (LC_TIME, NLW(ABMON_1) + tp->tm_mon))
> +  ((const CHAR_T *) (tp->tm_mon < 0 || tp->tm_mon > 11 ?		      \
> +		     "?" : _NL_CURRENT (LC_TIME, NLW(ABMON_1) + tp->tm_mon)))
>  # define f_month \
> -  ((const CHAR_T *) _NL_CURRENT (LC_TIME, NLW(MON_1) + tp->tm_mon))
> +  ((const CHAR_T *) (tp->tm_mon < 0 || tp->tm_mon > 11 ?		      \
> +		     "?" : _NL_CURRENT (LC_TIME, NLW(MON_1) + tp->tm_mon)))

Line break before operator, not after.

Andreas.
diff mbox

Patch

diff --git a/time/strftime_l.c b/time/strftime_l.c
index b48ef34..619e4e3 100644
--- a/time/strftime_l.c
+++ b/time/strftime_l.c
@@ -510,13 +510,17 @@  __strftime_internal (s, maxsize, format, tp, tzset_called ut_argument
      only a few elements.  Dereference the pointers only if the format
      requires this.  Then it is ok to fail if the pointers are invalid.  */
 # define a_wkday \
-  ((const CHAR_T *) _NL_CURRENT (LC_TIME, NLW(ABDAY_1) + tp->tm_wday))
+  ((const CHAR_T *) (tp->tm_wday < 0 || tp->tm_wday > 6 ?		      \
+		     "?" : _NL_CURRENT (LC_TIME, NLW(ABDAY_1) + tp->tm_wday)))
 # define f_wkday \
-  ((const CHAR_T *) _NL_CURRENT (LC_TIME, NLW(DAY_1) + tp->tm_wday))
+  ((const CHAR_T *) (tp->tm_wday < 0 || tp->tm_wday > 6 ?		      \
+		     "?" : _NL_CURRENT (LC_TIME, NLW(DAY_1) + tp->tm_wday)))
 # define a_month \
-  ((const CHAR_T *) _NL_CURRENT (LC_TIME, NLW(ABMON_1) + tp->tm_mon))
+  ((const CHAR_T *) (tp->tm_mon < 0 || tp->tm_mon > 11 ?		      \
+		     "?" : _NL_CURRENT (LC_TIME, NLW(ABMON_1) + tp->tm_mon)))
 # define f_month \
-  ((const CHAR_T *) _NL_CURRENT (LC_TIME, NLW(MON_1) + tp->tm_mon))
+  ((const CHAR_T *) (tp->tm_mon < 0 || tp->tm_mon > 11 ?		      \
+		     "?" : _NL_CURRENT (LC_TIME, NLW(MON_1) + tp->tm_mon)))
 # define ampm \
   ((const CHAR_T *) _NL_CURRENT (LC_TIME, tp->tm_hour > 11		      \
 				 ? NLW(PM_STR) : NLW(AM_STR)))
@@ -526,8 +530,10 @@  __strftime_internal (s, maxsize, format, tp, tzset_called ut_argument
 # define ap_len STRLEN (ampm)
 #else
 # if !HAVE_STRFTIME
-#  define f_wkday (weekday_name[tp->tm_wday])
-#  define f_month (month_name[tp->tm_mon])
+#  define f_wkday (tp->tm_wday < 0 || tp->tm_wday > 6 ? \
+		   "?" : weekday_name[tp->tm_wday])
+#  define f_month (tp->tm_mon < 0 || tp->tm_mon > 11 ?	\
+		   "?" : month_name[tp->tm_mon])
 #  define a_wkday f_wkday
 #  define a_month f_month
 #  define ampm (L_("AMPM") + 2 * (tp->tm_hour > 11))
@@ -1321,7 +1327,7 @@  __strftime_internal (s, maxsize, format, tp, tzset_called ut_argument
 		  *tzset_called = true;
 		}
 # endif
-	      zone = tzname[tp->tm_isdst];
+	      zone = tp->tm_isdst <= 1 ? tzname[tp->tm_isdst] : "?";
 	    }
 #endif
 	  if (! zone)
diff --git a/time/tst-strftime.c b/time/tst-strftime.c
index 374fba4..5a592c2 100644
--- a/time/tst-strftime.c
+++ b/time/tst-strftime.c
@@ -4,6 +4,24 @@ 
 #include <time.h>
 
 
+static int
+do_bz18985 (void)
+{
+  char buf[1000];
+  struct tm ttm;
+
+  memset (&ttm, 1, sizeof (ttm));
+  ttm.tm_zone = NULL;  /* Dereferenced directly if non-NULL.  */
+  strftime (buf, sizeof (buf), "%a %A %b %B %c %z %Z", &ttm);
+
+  /* Check negative values as well.  */
+  memset (&ttm, 0xFF, sizeof (ttm));
+  ttm.tm_zone = NULL;  /* Dereferenced directly if non-NULL.  */
+  strftime (buf, sizeof (buf), "%a %A %b %B %c %z %Z", &ttm);
+
+  return 0;
+}
+
 static struct
 {
   const char *fmt;
@@ -104,7 +122,7 @@  do_test (void)
 	}
     }
 
-  return result;
+  return result + do_bz18985 ();
 }
 
 #define TEST_FUNCTION do_test ()