[Author Prev][Author Next][Thread Prev][Thread Next][Author Index][Thread Index]

[or-cvs] r18761: {tor} Actually use tor_sscanf() to parse untrusted input. (in tor/trunk: . src/common src/or)

To: or-cvs@xxxxxxxxxxxxx
Subject: [or-cvs] r18761: {tor} Actually use tor_sscanf() to parse untrusted input. (in tor/trunk: . src/common src/or)
From: nickm@xxxxxxxx
Date: Tue, 3 Mar 2009 13:02:36 -0500 (EST)
Delivered-to: archiver@xxxxxxxx
Delivered-to: or-cvs-outgoing@xxxxxxxx
Delivered-to: or-cvs@xxxxxxxx
Delivery-date: Tue, 03 Mar 2009 13:02:38 -0500
Reply-to: or-dev@xxxxxxxxxxxxx
Sender: owner-or-cvs@xxxxxxxxxxxxx

Author: nickm
Date: 2009-03-03 13:02:36 -0500 (Tue, 03 Mar 2009)
New Revision: 18761

Modified:
   tor/trunk/ChangeLog
   tor/trunk/src/common/compat.c
   tor/trunk/src/common/util.c
   tor/trunk/src/or/directory.c
   tor/trunk/src/or/test.c
Log:
Actually use tor_sscanf() to parse untrusted input.

Modified: tor/trunk/ChangeLog
===================================================================
--- tor/trunk/ChangeLog	2009-03-03 18:02:31 UTC (rev 18760)
+++ tor/trunk/ChangeLog	2009-03-03 18:02:36 UTC (rev 18761)
@@ -39,6 +39,8 @@
     - Do not assume that a stack-allocated character array will be
       64-bit aligned on platforms that demand that uint64_t access is
       aligned.  Possible fix for bug 604.
+    - Parse dates and IPv4 addresses in a locale- and libc-independent
+      manner, to avoid platform-dependent behavior on malformed input.
 
   o Minor features:
     - On Linux, use the prctl call to re-enable core dumps when the user

Modified: tor/trunk/src/common/compat.c
===================================================================
--- tor/trunk/src/common/compat.c	2009-03-03 18:02:31 UTC (rev 18760)
+++ tor/trunk/src/common/compat.c	2009-03-03 18:02:36 UTC (rev 18761)
@@ -1296,14 +1296,14 @@
 int
 tor_inet_aton(const char *str, struct in_addr* addr)
 {
-  int a,b,c,d;
+  unsigned a,b,c,d;
   char more;
-  if (sscanf(str, "%d.%d.%d.%d%c", &a,&b,&c,&d,&more) != 4)
+  if (tor_sscanf(str, "%3u.%3u.%3u.%3u%c", &a,&b,&c,&d,&more) != 4)
     return 0;
-  if (a < 0 || a > 255) return 0;
-  if (b < 0 || b > 255) return 0;
-  if (c < 0 || c > 255) return 0;
-  if (d < 0 || d > 255) return 0;
+  if (a > 255) return 0;
+  if (b > 255) return 0;
+  if (c > 255) return 0;
+  if (d > 255) return 0;
   addr->s_addr = htonl((a<<24) | (b<<16) | (c<<8) | d);
   return 1;
 }
@@ -1421,7 +1421,7 @@
     else if (!dot)
       eow = src+strlen(src);
     else {
-      int byte1,byte2,byte3,byte4;
+      unsigned byte1,byte2,byte3,byte4;
       char more;
       for (eow = dot-1; eow >= src && TOR_ISDIGIT(*eow); --eow)
         ;
@@ -1429,13 +1429,11 @@
 
       /* We use "scanf" because some platform inet_aton()s are too lax
        * about IPv4 addresses of the form "1.2.3" */
-      if (sscanf(eow, "%d.%d.%d.%d%c", &byte1,&byte2,&byte3,&byte4,&more) != 4)
+      if (tor_sscanf(eow, "%3u.%3u.%3u.%3u%c",
+                     &byte1,&byte2,&byte3,&byte4,&more) != 4)
         return 0;
 
-      if (byte1 > 255 || byte1 < 0 ||
-          byte2 > 255 || byte2 < 0 ||
-          byte3 > 255 || byte3 < 0 ||
-          byte4 > 255 || byte4 < 0)
+      if (byte1 > 255 || byte2 > 255 || byte3 > 255 || byte4 > 255)
         return 0;
 
       words[6] = (byte1<<8) | byte2;

Modified: tor/trunk/src/common/util.c
===================================================================
--- tor/trunk/src/common/util.c	2009-03-03 18:02:31 UTC (rev 18760)
+++ tor/trunk/src/common/util.c	2009-03-03 18:02:36 UTC (rev 18761)
@@ -1095,18 +1095,30 @@
   char month[4];
   char weekday[4];
   int i, m;
+  unsigned tm_mday, tm_year, tm_hour, tm_min, tm_sec;
 
   if (strlen(buf) != RFC1123_TIME_LEN)
     return -1;
   memset(&tm, 0, sizeof(tm));
-  if (sscanf(buf, "%3s, %d %3s %d %d:%d:%d GMT", weekday,
-             &tm.tm_mday, month, &tm.tm_year, &tm.tm_hour,
-             &tm.tm_min, &tm.tm_sec) < 7) {
+  if (tor_sscanf(buf, "%3s, %2u %3s %u %2u:%2u:%2u GMT", weekday,
+             &tm_mday, month, &tm_year, &tm_hour,
+             &tm_min, &tm_sec) < 7) {
     char *esc = esc_for_log(buf);
     log_warn(LD_GENERAL, "Got invalid RFC1123 time %s", esc);
     tor_free(esc);
     return -1;
   }
+  if (tm_mday > 31 || tm_hour > 23 || tm_min > 59 || tm_sec > 61) {
+    char *esc = esc_for_log(buf);
+    log_warn(LD_GENERAL, "Got invalid RFC1123 time %s", esc);
+    tor_free(esc);
+    return -1;
+  }
+  tm.tm_mday = (int)tm_mday;
+  tm.tm_year = (int)tm_year;
+  tm.tm_hour = (int)tm_hour;
+  tm.tm_min = (int)tm_min;
+  tm.tm_sec = (int)tm_sec;
 
   m = -1;
   for (i = 0; i < 12; ++i) {
@@ -1166,19 +1178,20 @@
 parse_iso_time(const char *cp, time_t *t)
 {
   struct tm st_tm;
-#ifdef HAVE_STRPTIME
-  if (!strptime(cp, "%Y-%m-%d %H:%M:%S", &st_tm)) {
-    log_warn(LD_GENERAL, "ISO time was unparseable by strptime"); return -1;
-  }
-#else
   unsigned int year=0, month=0, day=0, hour=100, minute=100, second=100;
-  if (sscanf(cp, "%u-%u-%u %u:%u:%u", &year, &month,
+  if (tor_sscanf(cp, "%u-%2u-%2u %2u:%2u:%2u", &year, &month,
                 &day, &hour, &minute, &second) < 6) {
-    log_warn(LD_GENERAL, "ISO time was unparseable"); return -1;
+    char *esc = esc_for_log(cp);
+    log_warn(LD_GENERAL, "ISO time %s was unparseable", esc);
+    tor_free(esc);
+    return -1;
   }
   if (year < 1970 || month < 1 || month > 12 || day < 1 || day > 31 ||
           hour > 23 || minute > 59 || second > 61) {
-    log_warn(LD_GENERAL, "ISO time was nonsensical"); return -1;
+    char *esc = esc_for_log(cp);
+    log_warn(LD_GENERAL, "ISO time %s was nonsensical", esc);
+    tor_free(esc);
+    return -1;
   }
   st_tm.tm_year = year-1900;
   st_tm.tm_mon = month-1;
@@ -1186,7 +1199,7 @@
   st_tm.tm_hour = hour;
   st_tm.tm_min = minute;
   st_tm.tm_sec = second;
-#endif
+
   if (st_tm.tm_year < 70) {
     char *esc = esc_for_log(cp);
     log_warn(LD_GENERAL, "Got invalid ISO time %s. (Before 1970)", esc);
@@ -1206,6 +1219,7 @@
   char month[4];
   char wkday[4];
   int i;
+  unsigned tm_mday, tm_year, tm_hour, tm_min, tm_sec;
 
   tor_assert(tm);
   memset(tm, 0, sizeof(*tm));
@@ -1213,28 +1227,33 @@
   /* First, try RFC1123 or RFC850 format: skip the weekday.  */
   if ((cp = strchr(date, ','))) {
     ++cp;
-    if (sscanf(date, "%2d %3s %4d %2d:%2d:%2d GMT",
-               &tm->tm_mday, month, &tm->tm_year,
-               &tm->tm_hour, &tm->tm_min, &tm->tm_sec) == 6) {
+    if (tor_sscanf(date, "%2u %3s %4u %2u:%2u:%2u GMT",
+               &tm_mday, month, &tm_year,
+               &tm_hour, &tm_min, &tm_sec) == 6) {
       /* rfc1123-date */
-      tm->tm_year -= 1900;
-    } else if (sscanf(date, "%2d-%3s-%2d %2d:%2d:%2d GMT",
-                      &tm->tm_mday, month, &tm->tm_year,
-                      &tm->tm_hour, &tm->tm_min, &tm->tm_sec) == 6) {
+      tm_year -= 1900;
+    } else if (tor_sscanf(date, "%2u-%3s-%2u %2u:%2u:%2u GMT",
+                      &tm_mday, month, &tm_year,
+                      &tm_hour, &tm_min, &tm_sec) == 6) {
       /* rfc850-date */
     } else {
       return -1;
     }
   } else {
     /* No comma; possibly asctime() format. */
-    if (sscanf(date, "%3s %3s %2d %2d:%2d:%2d %4d",
-               wkday, month, &tm->tm_mday,
-               &tm->tm_hour, &tm->tm_min, &tm->tm_sec, &tm->tm_year) == 7) {
-      tm->tm_year -= 1900;
+    if (tor_sscanf(date, "%3s %3s %2u %2u:%2u:%2u %4u",
+               wkday, month, &tm_mday,
+               &tm_hour, &tm_min, &tm_sec, &tm_year) == 7) {
+      tm_year -= 1900;
     } else {
       return -1;
     }
   }
+  tm->tm_mday = (int)tm_mday;
+  tm->tm_year = (int)tm_year;
+  tm->tm_hour = (int)tm_hour;
+  tm->tm_min = (int)tm_min;
+  tm->tm_sec = (int)tm_sec;
 
   month[3] = '\0';
   /* Okay, now decode the month. */
@@ -2202,7 +2221,7 @@
 {
   unsigned result = 0;
   int scanned_so_far = 0;
-  if (!bufp || !*bufp)
+  if (!bufp || !*bufp || !out)
     return -1;
   if (width<0)
     width=MAX_SCANF_WIDTH;
@@ -2228,7 +2247,7 @@
 scan_string(const char **bufp, char *out, int width)
 {
   int scanned_so_far = 0;
-  if (!bufp || width < 0)
+  if (!bufp || !out || width < 0)
     return -1;
   while (**bufp && ! TOR_ISSPACE(**bufp) && scanned_so_far < width) {
     *out++ = *(*bufp)++;
@@ -2312,7 +2331,7 @@
  * and store the results in the corresponding argument fields.  Differs from
  * sscanf in that it: Only handles %u and %Ns.  Does not handle arbitrarily
  * long widths. %u does not consume any space.  Is locale-independent.
- * Returns -1 on malformed  */
+ * Returns -1 on malformed patterns. */
 int
 tor_sscanf(const char *buf, const char *pattern, ...)
 {

Modified: tor/trunk/src/or/directory.c
===================================================================
--- tor/trunk/src/or/directory.c	2009-03-03 18:02:31 UTC (rev 18760)
+++ tor/trunk/src/or/directory.c	2009-03-03 18:02:36 UTC (rev 18761)
@@ -1247,7 +1247,7 @@
 parse_http_response(const char *headers, int *code, time_t *date,
                     compress_method_t *compression, char **reason)
 {
-  int n1, n2;
+  unsigned n1, n2;
   char datestr[RFC1123_TIME_LEN+1];
   smartlist_t *parsed_headers;
   tor_assert(headers);
@@ -1255,7 +1255,7 @@
 
   while (TOR_ISSPACE(*headers)) headers++; /* tolerate leading whitespace */
 
-  if (sscanf(headers, "HTTP/1.%d %d", &n1, &n2) < 2 ||
+  if (tor_sscanf(headers, "HTTP/1.%u %u", &n1, &n2) < 2 ||
       (n1 != 0 && n1 != 1) ||
       (n2 < 100 || n2 >= 600)) {
     log_warn(LD_HTTP,"Failed to parse header %s",escaped(headers));

Modified: tor/trunk/src/or/test.c
===================================================================
--- tor/trunk/src/or/test.c	2009-03-03 18:02:31 UTC (rev 18760)
+++ tor/trunk/src/or/test.c	2009-03-03 18:02:36 UTC (rev 18761)
@@ -2796,6 +2796,11 @@
   test_eq(u1, 12u);
   test_eq(u2, 3u);
   test_eq(u3, 99u);
+
+  r = tor_sscanf("99% fresh", "%3u%% fresh", &u1); /* percents are scannable.*/
+  test_eq(r, 1);
+  test_eq(u1, 99);
+
   r = tor_sscanf("hello", "%s", s1); /* %s needs a number. */
   test_eq(r, -1);

Prev by Author: [or-cvs] r18760: {tor} Add a simple locale-independent no-surprises sscanf replacem (in tor/trunk/src: common or)
Next by Author: [or-cvs] r18809: {tor} Build correctly from outside main source tree. Patch from Mi (tor/trunk)
Previous by thread: [or-cvs] r18760: {tor} Add a simple locale-independent no-surprises sscanf replacem (in tor/trunk/src: common or)
Next by thread: [or-cvs] r18762: {website} Add Vienna Chamber of Commerce article. (website/trunk/en)
Index(es):
- Author
- Thread