Fix UB in jv2tm()

nicowilliams · Aug 29, 2023 · 821b125 · 821b125
1 parent 6fc581b
commit 821b125
Show file tree

Hide file tree

Showing 2 changed files with 59 additions and 29 deletions.
diff --git a/src/builtin.c b/src/builtin.c
@@ -1446,53 +1446,76 @@ static jv f_strptime(jq_state *jq, jv a, jv b) {
   return r;
 }
 
-#define TO_TM_FIELD(t, j, i)                    \
-    do {                                        \
-      jv n = jv_array_get(jv_copy(j), (i));     \
-      if (jv_get_kind(n) != (JV_KIND_NUMBER)) { \
-        jv_free(n);                             \
-        jv_free(j);                             \
-        return 0;                               \
-      }                                         \
-      t = jv_number_value(n);                   \
-      jv_free(n);                               \
-    } while (0)
+static int toint(jv v, int imin, int imax, int ifnan) {
+  if (jv_get_kind(v) != (JV_KIND_NUMBER) || jvp_number_is_nan(v)) {
+    jv_free(v);
+    return ifnan;
+  }
+  if (jvp_number_is_nan(v)) {
+    jv_free(v);
+    return ifnan;
+  }
+  double n = jv_number_value(v);
+  jv_free(v);
+  if (isinf(n)) {
+    if (n < 0.0)
+      return imin;
+    return imax;
+  }
+  if (n < imin)
+    return imin;
+  if (n > imax)
+    return imax;
+  return (int)n;
+}
 
 static int jv2tm(jv a, struct tm *tm, double *frac, char **freeme) {
   int ret = 1;
 
   *freeme = NULL;
   memset(tm, 0, sizeof(*tm));
-  TO_TM_FIELD(tm->tm_year, a, 0);
+  if (   (tm->tm_year = toint(jv_array_get(jv_copy(a), 0),
+                              1970, 10001, 1969))                       < 1970
+      ||  tm->tm_year                                                   > 10000
+      || (tm->tm_mon  = toint(jv_array_get(jv_copy(a), 1), -1, 12,  -1)) < 0
+      ||  tm->tm_mon                                                    > 11
+      || (tm->tm_mday = toint(jv_array_get(jv_copy(a), 2),  0, 32,   0)) < 1
+      ||  tm->tm_mday                                                   > 31
+      || (tm->tm_hour = toint(jv_array_get(jv_copy(a), 3), -1, 24,  -1)) < 0
+      ||  tm->tm_hour                                                   > 23
+      || (tm->tm_min  = toint(jv_array_get(jv_copy(a), 4), -1, 60,  -1)) < 0
+      ||  tm->tm_min                                                    > 59
+      || (tm->tm_sec  = toint(jv_array_get(jv_copy(a), 5), -1, 61,  -1)) < 0
+      ||  tm->tm_sec                                                    > 60
+      || (tm->tm_wday = toint(jv_array_get(jv_copy(a), 6), -1, 7,   -1)) < 0
+      ||  tm->tm_wday                                                   > 6
+      || (tm->tm_yday = toint(jv_array_get(jv_copy(a), 7), -1, 366, -1)) < 0
+      ||  tm->tm_yday                                                   > 365) {
+    jv_free(a);
+    return 0;
+  }
   tm->tm_year -= 1900;
-  TO_TM_FIELD(tm->tm_mon,  a, 1);
-  TO_TM_FIELD(tm->tm_mday, a, 2);
-  TO_TM_FIELD(tm->tm_hour, a, 3);
-  TO_TM_FIELD(tm->tm_min,  a, 4);
-  TO_TM_FIELD(tm->tm_sec,  a, 5);
   if (frac) {
+    // We already know this is a number, not a NaN, and in range.  We would
+    // have returned already otherwise.
+    jv n = jv_array_get(jv_copy(a), 5);
     double d = jv_number_value(jv_array_get(jv_copy(a), 5));
-
-    *frac = d - floor(d);
+    jv_free(n);
+    *frac = d - tm->tm_sec;
   }
-  TO_TM_FIELD(tm->tm_wday, a, 6);
-  TO_TM_FIELD(tm->tm_yday, a, 7);
   jv v = jv_array_get(jv_copy(a), 8);
   switch (jv_get_kind(v)) {
   case JV_KIND_INVALID: break;
   case JV_KIND_FALSE: break;
   case JV_KIND_TRUE: tm->tm_isdst = 1; break;
-  case JV_KIND_NUMBER: tm->tm_isdst = !!(int)jv_number_value(v); break;
+  case JV_KIND_NUMBER: tm->tm_isdst = toint(v, 0, 1, 0); break;
   default: ret = 0; break;
   }
   jv_free(v);
 #ifdef HAVE_TM_TM_GMTOFF
-  v = jv_array_get(jv_copy(a), 9);
-  if (jv_get_kind(v) == JV_KIND_NUMBER)
-    tm->tm_gmtoff = jv_number_value(v);
-  else if (jv_is_valid(v))
+  tm->tm_gmtoff = toint(jv_array_get(jv_copy(a), 9), -12 * 3600 - 1, 12 * 3600 + 1, 0);
+  if (tm->tm_gmtoff < -12 * 3600 || tm->tm_gmtoff > 12 * 3600)
     ret = 0;
-  jv_free(v);
 #endif
 #ifdef HAVE_STRUCT_TM_TM_ZONE
   v = jv_array_get(jv_copy(a), 10);
@@ -1522,8 +1545,6 @@ static int jv2tm(jv a, struct tm *tm, double *frac, char **freeme) {
   return ret;
 }
 
-#undef TO_TM_FIELD
-
 static jv f_mktime(jq_state *jq, jv a) {
   if (jv_get_kind(a) != JV_KIND_ARRAY)
     return ret_error(a, jv_string("mktime requires array inputs"));

diff --git a/tests/jq.test b/tests/jq.test
@@ -1580,6 +1580,15 @@ try mktime catch .
 ["a",1,2,3,4,5,6,7]
 "mktime requires parsed datetime inputs"
 
+# #2863
+[.[]|try (timegm | error) catch .]|[length,unique[]]
+[[1969],[10001],[1970,-1],[1970,12],[1970,11,0],[1970,11,32],[1970,11,30,-1],[1970,11,30,24],[1970,11,30,23,-1],[1970,11,31,23,60],[1970,11,30,23,59,-1],[1970,11,30,23,59,61],[1970,11,30,23,59,60,-1],[1970,11,30,23,59,60,7],[1970,11,30,23,59,60,4],[1970,11,30,23,59,60,4,-1],[1970,11,30,23,59,60,4,366],[1970,11,30,23,59,60,4,364,true,-43201],[1970,11,30,23,59,60,4,364,true,43201],[1970,11,30,23,59,60,4,364,true,0,true]]
+[20,"timegm requires parsed datetime inputs"]
+
+[.[]|timegm]|length
+[[1970,11,30,23,59,60,4,364],[1970,11,30,23,59,60,4,364,true,-43200],[1970,11,30,23,59,60,4,364,true,43200],[1970,11,30,23,59,60,4,364,true,43200,"EDT"]]
+4
+
 # module system
 import "a" as foo; import "b" as bar; def fooa: foo::a; [fooa, bar::a, bar::b, foo::a]
 null