Wireshark · Ethereal-dev: [Ethereal-dev] patch for localtime(3) and gmtime(3) returning NULL

[Scott Renfro <scott@xxxxxxxxxx>]

In all but one place where ethereal uses localtime(3) and gmtime(3), the
code assumes that a valid pointer was returned.  This is not always the
case.  On some platforms, these functions return NULL as an error
indicator, either because they don't know the timezone (in the case of
localtime), or the time provided wasn't considered representable.

In particular, the Microsoft CRT returns null when the time_t value
passed is negative.  Although this isn't likely to happen for a few
years as a result of reading the clock, when reading arbitrary data from
the network, anything is possible.  This lead to a null pointer
exception decoding such a frame with the latest release on win32 (no
problem -- other than a date in the year 1968 -- on FreeBSD.

This patch wraps the use of the pointer returned by localtime or gmtime
in an if/else that ensures the pointer is not NULL before dereferencing
it.  If the pointer is null, a default value is stored in the buffer.
For column functions, the value is the empty string.  For all other
functions (these are either in epan or in dissectors, but the resulting
string ends up in the display where there's more room than the column),
the string "Not representable" is stored.  I think I hit all calls to
these functions in the source tree (at least cscope says I did ;-)

cheers,
--Scott

-- 
Scott Renfro <scott@xxxxxxxxxx>                          +1 650 862 4206

Index: packet-diameter.c
===================================================================
RCS file: /cvsroot/ethereal/packet-diameter.c,v
retrieving revision 1.23
diff -u -r1.23 packet-diameter.c
--- packet-diameter.c	2001/06/18 02:17:45	1.23
+++ packet-diameter.c	2001/07/15 06:32:31
@@ -702,12 +702,16 @@
 #endif
 		case DIAMETER_TIME:
 		{
-			struct tm lt;
+			struct tm *lt;
 			intval=pntohl(input);
 			intval -= NTP_TIME_DIFF;
-			lt=*localtime((time_t *)&intval);
-			strftime(buffer, 1024, 
-			    "%a, %d %b %Y %H:%M:%S %z",&lt);
+			lt=localtime((time_t *)&intval);
+			if (lt != NULL) {
+			  strftime(buffer, 1024, 
+			           "%a, %d %b %Y %H:%M:%S %z",lt);
+			} else {
+			  strncpy(buffer, "Not representable", 1024);
+			}
 		}
 		default:
 			/* Do nothing */
Index: packet-ntp.c
===================================================================
RCS file: /cvsroot/ethereal/packet-ntp.c,v
retrieving revision 1.28
diff -u -r1.28 packet-ntp.c
--- packet-ntp.c	2001/06/18 02:17:50	1.28
+++ packet-ntp.c	2001/07/15 06:32:35
@@ -236,10 +236,14 @@
 	} else {
 		temptime = tempstmp - (guint32) NTP_BASETIME;
 		bd = gmtime(&temptime);
-		fractime = bd->tm_sec + tempfrac / 4294967296.0;
-		snprintf(buff, NTP_TS_SIZE, "%04d-%02d-%02d %02d:%02d:%07.4f UTC",
-			 bd->tm_year + 1900, bd->tm_mon + 1, bd->tm_mday,
-			 bd->tm_hour, bd->tm_min, fractime);
+        if (bd != NULL) {
+          fractime = bd->tm_sec + tempfrac / 4294967296.0;
+          snprintf(buff, NTP_TS_SIZE, "%04d-%02d-%02d %02d:%02d:%07.4f UTC",
+                   bd->tm_year + 1900, bd->tm_mon + 1, bd->tm_mday,
+                   bd->tm_hour, bd->tm_min, fractime);
+        } else {
+          strncpy(buff, "Not representable", NTP_TS_SIZE);
+        }
 	}
 	return buff;
 }
Index: packet-srvloc.c
===================================================================
RCS file: /cvsroot/ethereal/packet-srvloc.c,v
retrieving revision 1.24
diff -u -r1.24 packet-srvloc.c
--- packet-srvloc.c	2001/06/18 02:17:53	1.24
+++ packet-srvloc.c	2001/07/15 06:32:39
@@ -193,12 +193,16 @@
     
     seconds = tvb_get_ntohl(tvb, offset) - 2208988800ul;
     stamp = gmtime(&seconds);
-    floatsec = stamp->tm_sec + tvb_get_ntohl(tvb, offset + 4) / 4294967296.0;
-    proto_tree_add_text(tree, tvb, offset, 8,
-			"Timestamp: %04d-%02d-%02d %02d:%02d:%07.4f UTC",
-			stamp->tm_year + 1900, stamp->tm_mon + 1,
-			stamp->tm_mday, stamp->tm_hour, stamp->tm_min,
-			floatsec);
+    if (stamp != NULL) {
+      floatsec = stamp->tm_sec + tvb_get_ntohl(tvb, offset + 4) / 4294967296.0;
+      proto_tree_add_text(tree, tvb, offset, 8,
+                          "Timestamp: %04d-%02d-%02d %02d:%02d:%07.4f UTC",
+                          stamp->tm_year + 1900, stamp->tm_mon + 1,
+                          stamp->tm_mday, stamp->tm_hour, stamp->tm_min,
+                          floatsec);
+    } else {
+      proto_tree_add_text(tree, tvb, offset, 8, "Timestamp not representable");
+    }
     proto_tree_add_text(tree, tvb, offset + 8, 2, "Block Structure Desciptor: %u",
 			tvb_get_ntohs(tvb, offset + 8));
     length = tvb_get_ntohs(tvb, offset + 10);
Index: epan/column-utils.c
===================================================================
RCS file: /cvsroot/ethereal/epan/column-utils.c,v
retrieving revision 1.3
diff -u -r1.3 epan/column-utils.c
--- epan/column-utils.c	2001/04/02 10:38:26	1.3
+++ epan/column-utils.c	2001/07/15 06:32:42
@@ -248,15 +248,19 @@
 
   then = fd->abs_secs;
   tmp = localtime(&then);
-  snprintf(fd->cinfo->col_buf[col], COL_MAX_LEN,
-    "%04d-%02d-%02d %02d:%02d:%02d.%04ld",
-    tmp->tm_year + 1900,
-    tmp->tm_mon + 1,
-    tmp->tm_mday,
-    tmp->tm_hour,
-    tmp->tm_min,
-    tmp->tm_sec,
-    (long)fd->abs_usecs/100);
+  if (tmp != NULL) {
+    snprintf(fd->cinfo->col_buf[col], COL_MAX_LEN,
+             "%04d-%02d-%02d %02d:%02d:%02d.%04ld",
+             tmp->tm_year + 1900,
+             tmp->tm_mon + 1,
+             tmp->tm_mday,
+             tmp->tm_hour,
+             tmp->tm_min,
+             tmp->tm_sec,
+             (long)fd->abs_usecs/100);
+  } else {
+    fd->cinfo->col_buf[col][0] = '\0';
+  }
   fd->cinfo->col_data[col] = fd->cinfo->col_buf[col];
 }
 
@@ -286,11 +290,15 @@
 
   then = fd->abs_secs;
   tmp = localtime(&then);
-  snprintf(fd->cinfo->col_buf[col], COL_MAX_LEN, "%02d:%02d:%02d.%04ld",
-    tmp->tm_hour,
-    tmp->tm_min,
-    tmp->tm_sec,
-    (long)fd->abs_usecs/100);
+  if (tmp != NULL) {
+    snprintf(fd->cinfo->col_buf[col], COL_MAX_LEN, "%02d:%02d:%02d.%04ld",
+             tmp->tm_hour,
+             tmp->tm_min,
+             tmp->tm_sec,
+             (long)fd->abs_usecs/100);
+  } else {
+    fd->cinfo->col_buf[col][0] = '\0';
+  }
   fd->cinfo->col_data[col] = fd->cinfo->col_buf[col];
 }
 
Index: epan/to_str.c
===================================================================
RCS file: /cvsroot/ethereal/epan/to_str.c,v
retrieving revision 1.9
diff -u -r1.9 epan/to_str.c
--- epan/to_str.c	2001/07/13 00:27:51	1.9
+++ epan/to_str.c	2001/07/15 06:32:45
@@ -411,15 +411,18 @@
         }
 
         tmp = localtime(&abs_time->tv_sec);
-        sprintf(cur, "%s %2d, %d %02d:%02d:%02d.%06ld",
-            mon_names[tmp->tm_mon],
-            tmp->tm_mday,
-            tmp->tm_year + 1900,
-            tmp->tm_hour,
-            tmp->tm_min,
-            tmp->tm_sec,
-            (long)abs_time->tv_usec);
-
+        if (tmp) {
+          sprintf(cur, "%s %2d, %d %02d:%02d:%02d.%06ld",
+                  mon_names[tmp->tm_mon],
+                  tmp->tm_mday,
+                  tmp->tm_year + 1900,
+                  tmp->tm_hour,
+                  tmp->tm_min,
+                  tmp->tm_sec,
+                  (long)abs_time->tv_usec);
+        } else {
+          strncpy(cur, "Not representable", sizeof(str[0]));
+        }
         return cur;
 }
 
Index: wiretap/netmon.c
===================================================================
RCS file: /cvsroot/ethereal/wiretap/netmon.c,v
retrieving revision 1.38
diff -u -r1.38 wiretap/netmon.c
--- wiretap/netmon.c	2001/07/13 00:55:58	1.38
+++ wiretap/netmon.c	2001/07/15 06:32:50
@@ -635,14 +635,24 @@
 
 	file_hdr.network = htoles(wtap_encap[wdh->encap]);
 	tm = localtime(&netmon->first_record_time.tv_sec);
-	file_hdr.ts_year = htoles(1900 + tm->tm_year);
-	file_hdr.ts_month = htoles(tm->tm_mon + 1);
-	file_hdr.ts_dow = htoles(tm->tm_wday);
-	file_hdr.ts_day = htoles(tm->tm_mday);
-	file_hdr.ts_hour = htoles(tm->tm_hour);
-	file_hdr.ts_min = htoles(tm->tm_min);
-	file_hdr.ts_sec = htoles(tm->tm_sec);
-	file_hdr.ts_msec = htoles(netmon->first_record_time.tv_usec/1000);
+    if (tm != NULL) {
+      file_hdr.ts_year  = htoles(1900 + tm->tm_year);
+      file_hdr.ts_month = htoles(tm->tm_mon + 1);
+      file_hdr.ts_dow   = htoles(tm->tm_wday);
+      file_hdr.ts_day   = htoles(tm->tm_mday);
+      file_hdr.ts_hour  = htoles(tm->tm_hour);
+      file_hdr.ts_min   = htoles(tm->tm_min);
+      file_hdr.ts_sec   = htoles(tm->tm_sec);
+    } else {
+      file_hdr.ts_year  = htoles(1900 + 0);
+      file_hdr.ts_month = htoles(0 + 1);
+      file_hdr.ts_dow   = htoles(0);
+      file_hdr.ts_day   = htoles(0);
+      file_hdr.ts_hour  = htoles(0);
+      file_hdr.ts_min   = htoles(0);
+      file_hdr.ts_sec   = htoles(0);
+    }      
+    file_hdr.ts_msec  = htoles(netmon->first_record_time.tv_usec/1000);
 		/* XXX - what about rounding? */
 	file_hdr.frametableoffset = htolel(netmon->frame_table_offset);
 	file_hdr.frametablelength =
Index: wiretap/ngsniffer.c
===================================================================
RCS file: /cvsroot/ethereal/wiretap/ngsniffer.c,v
retrieving revision 1.64
diff -u -r1.64 wiretap/ngsniffer.c
--- wiretap/ngsniffer.c	2001/07/06 00:17:36	1.64
+++ wiretap/ngsniffer.c	2001/07/15 06:33:01
@@ -1204,11 +1204,16 @@
     if (priv->first_frame) {
 	priv->first_frame=FALSE;
 	tm = localtime(&phdr->ts.tv_sec);
-	start_date = (tm->tm_year - (1980 - 1900)) << 9;
-	start_date |= (tm->tm_mon + 1) << 5;
-	start_date |= tm->tm_mday;
-	/* record the start date, not the start time */
-	priv->start = phdr->ts.tv_sec - (3600*tm->tm_hour + 60*tm->tm_min + tm->tm_sec);
+	if (tm != NULL) {
+	  start_date = (tm->tm_year - (1980 - 1900)) << 9;
+	  start_date |= (tm->tm_mon + 1) << 5;
+	  start_date |= tm->tm_mday;
+	  /* record the start date, not the start time */
+	  priv->start = phdr->ts.tv_sec - (3600*tm->tm_hour + 60*tm->tm_min + tm->tm_sec);
+	} else {
+	  start_date = 0;
+	  priv->start = 0;
+	}
 
 	/* "sniffer" version ? */
 	maj_vers = 4;