From cbbc0c9c8615d4541d99a0e3ff2027b878893888 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Tue, 3 Mar 2009 18:02:36 +0000 Subject: [PATCH] Actually use tor_sscanf() to parse untrusted input. svn:r18761 --- ChangeLog | 2 ++ src/common/compat.c | 22 ++++++++--------- src/common/util.c | 71 +++++++++++++++++++++++++++++++++-------------------- src/or/directory.c | 4 +-- src/or/test.c | 5 ++++ 5 files changed, 64 insertions(+), 40 deletions(-) diff --git a/ChangeLog b/ChangeLog index 0ff79cfbb2..8061c9ee17 100644 --- a/ChangeLog +++ b/ChangeLog @@ -39,6 +39,8 @@ Changes in version 0.2.1.13-????? - 2009-02-?? - 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 diff --git a/src/common/compat.c b/src/common/compat.c index 8fd6d9bc15..cfd428133e 100644 --- a/src/common/compat.c +++ b/src/common/compat.c @@ -1296,14 +1296,14 @@ get_user_homedir(const char *username) 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 @@ tor_inet_pton(int af, const char *src, void *dst) 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 @@ tor_inet_pton(int af, const char *src, void *dst) /* 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; diff --git a/src/common/util.c b/src/common/util.c index 828b3ca038..6310811961 100644 --- a/src/common/util.c +++ b/src/common/util.c @@ -1095,18 +1095,30 @@ parse_rfc1123_time(const char *buf, time_t *t) 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 @@ int 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 @@ parse_iso_time(const char *cp, time_t *t) 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 @@ parse_http_time(const char *date, struct tm *tm) 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 @@ parse_http_time(const char *date, struct tm *tm) /* 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 @@ scan_unsigned(const char **bufp, unsigned *out, int width) { 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 @@ static int 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 @@ tor_vsscanf(const char *buf, const char *pattern, va_list ap) * 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, ...) { diff --git a/src/or/directory.c b/src/or/directory.c index 6fac10dd7d..efccb1ef42 100644 --- a/src/or/directory.c +++ b/src/or/directory.c @@ -1247,7 +1247,7 @@ int 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 @@ parse_http_response(const char *headers, int *code, time_t *date, 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)); diff --git a/src/or/test.c b/src/or/test.c index 46f717e893..ff05b1dafb 100644 --- a/src/or/test.c +++ b/src/or/test.c @@ -2796,6 +2796,11 @@ test_util_sscanf(void) 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); -- 2.11.4.GIT