From e37c1df9cd79bb8c540aded6ff35ea4cde8ddc60 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Wed, 9 Aug 2017 09:55:12 -0400 Subject: [PATCH 1/2] Don't use "0" as a "base" argument to tor_parse_*(). Telling these functions to autodetect the numeric base has lead to trouble in the past. Fixes bug 22469. Bugfix on 0.2.2.various. --- changes/bug22802 | 10 ++++++++++ src/common/procmon.c | 2 +- src/or/circuitstats.c | 4 ++-- src/or/dirserv.c | 2 +- 4 files changed, 14 insertions(+), 4 deletions(-) create mode 100644 changes/bug22802 diff --git a/changes/bug22802 b/changes/bug22802 new file mode 100644 index 0000000000..7255164fd4 --- /dev/null +++ b/changes/bug22802 @@ -0,0 +1,10 @@ + o Minor bugfixes (format strictness): + - Restrict several data formats to decimal. Previously, the + BuildTimeHistogram entries in the state file, the "bw=" entries in the + bandwidth authority file, and process IDs passed to the + __OwningControllerProcess option could all be specified in hex or octal + as well as in decimal. This was not an intentional feature. + Fixes bug 22802; bugfixes on 0.2.2.1-alpha, 0.2.2.2-alpha, and + 0.2.2.28-beta. + + diff --git a/src/common/procmon.c b/src/common/procmon.c index d49e7f18f5..5ef16c7268 100644 --- a/src/common/procmon.c +++ b/src/common/procmon.c @@ -71,7 +71,7 @@ parse_process_specifier(const char *process_spec, /* If we're lucky, long will turn out to be large enough to hold a * PID everywhere that Tor runs. */ - pid_l = tor_parse_long(process_spec, 0, 1, LONG_MAX, &pid_ok, &pspec_next); + pid_l = tor_parse_long(process_spec, 10, 1, LONG_MAX, &pid_ok, &pspec_next); /* Reserve room in the ‘process specifier’ for additional * (platform-specific) identifying information beyond the PID, to diff --git a/src/or/circuitstats.c b/src/or/circuitstats.c index 51d580a1a4..963892c9d1 100644 --- a/src/or/circuitstats.c +++ b/src/or/circuitstats.c @@ -939,7 +939,7 @@ circuit_build_times_parse_state(circuit_build_times_t *cbt, uint32_t count, k; build_time_t ms; int ok; - ms = (build_time_t)tor_parse_ulong(ms_str, 0, 0, + ms = (build_time_t)tor_parse_ulong(ms_str, 10, 0, CBT_BUILD_TIME_MAX, &ok, NULL); if (!ok) { log_warn(LD_GENERAL, "Unable to parse circuit build times: " @@ -949,7 +949,7 @@ circuit_build_times_parse_state(circuit_build_times_t *cbt, smartlist_free(args); break; } - count = (uint32_t)tor_parse_ulong(count_str, 0, 0, + count = (uint32_t)tor_parse_ulong(count_str, 10, 0, UINT32_MAX, &ok, NULL); if (!ok) { log_warn(LD_GENERAL, "Unable to parse circuit build times: " diff --git a/src/or/dirserv.c b/src/or/dirserv.c index e5654e3b90..01f70858ea 100644 --- a/src/or/dirserv.c +++ b/src/or/dirserv.c @@ -2700,7 +2700,7 @@ measured_bw_line_parse(measured_bw_line_t *out, const char *orig_line) } cp+=strlen("bw="); - out->bw_kb = tor_parse_long(cp, 0, 0, LONG_MAX, &parse_ok, &endptr); + out->bw_kb = tor_parse_long(cp, 10, 0, LONG_MAX, &parse_ok, &endptr); if (!parse_ok || (*endptr && !TOR_ISSPACE(*endptr))) { log_warn(LD_DIRSERV, "Invalid bandwidth in bandwidth file line: %s", escaped(orig_line)); From b88d00fea35630d96bf91bda362922af43730a04 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Wed, 9 Aug 2017 09:58:16 -0400 Subject: [PATCH 2/2] Don't fall back to _atoi64 We only did this on windows when building with MSVC 6 and earlier, which is now considered a screamingly bad idea. --- src/common/util.c | 8 -------- 1 file changed, 8 deletions(-) diff --git a/src/common/util.c b/src/common/util.c index 0858d17fe6..9cd9ebdc4d 100644 --- a/src/common/util.c +++ b/src/common/util.c @@ -1233,15 +1233,7 @@ tor_parse_uint64(const char *s, int base, uint64_t min, #ifdef HAVE_STRTOULL r = (uint64_t)strtoull(s, &endptr, base); #elif defined(_WIN32) -#if defined(_MSC_VER) && _MSC_VER < 1300 - tor_assert(base <= 10); - r = (uint64_t)_atoi64(s); - endptr = (char*)s; - while (TOR_ISSPACE(*endptr)) endptr++; - while (TOR_ISDIGIT(*endptr)) endptr++; -#else r = (uint64_t)_strtoui64(s, &endptr, base); -#endif #elif SIZEOF_LONG == 8 r = (uint64_t)strtoul(s, &endptr, base); #else