Provide constant limits for all consensus params

This addresses Nick's concern about doing non-constant bounds checking
inside networkstatus_get_param().

3 files changed, 38 insertions, 17 deletions

diff --git a/src/or/circuitbuild.c b/src/or/circuitbuild.c
index a8e9778789..3788959556 100644
--- a/src/or/circuitbuild.c
+++ b/src/or/circuitbuild.c
@@ -184,12 +184,19 @@ circuit_build_times_get_bw_scale(networkstatus_t *ns)
 static double
 circuit_build_times_close_quantile(void)
 {
-  return networkstatus_get_param(NULL, "cbtclosequantile",
+  int32_t param;
+  /* Cast is safe - circuit_build_times_quantile_cutoff() is capped */
+  int32_t min = (int)tor_lround(100*circuit_build_times_quantile_cutoff());
+  param = networkstatus_get_param(NULL, "cbtclosequantile",
              CBT_DEFAULT_CLOSE_QUANTILE,
-             /* Cast is safe, cbtquantile is capped at
-             * CBT_MAX_QUANTILE_CUTOFF. */
-             (int)tor_lround(100*circuit_build_times_quantile_cutoff()),
-             CBT_MAX_CLOSE_QUANTILE) / 100.0;
+             CBT_MIN_CLOSE_QUANTILE,
+             CBT_MAX_CLOSE_QUANTILE);
+  if (param < min) {
+    log_warn(LD_DIR, "Consensus parameter cbtclosequantile is "
+             "too small, raising to %d", min);
+    param = min;
+  }
+  return param / 100.0;
 }
 
 static int32_t
@@ -215,11 +222,17 @@ circuit_build_times_min_timeout(void)
 int32_t
 circuit_build_times_initial_timeout(void)
 {
-  int32_t num = networkstatus_get_param(NULL, "cbtinitialtimeout",
-                                        CBT_DEFAULT_TIMEOUT_INITIAL_VALUE,
-                                        circuit_build_times_min_timeout(),
-                                        CBT_MAX_TIMEOUT_INITIAL_VALUE);
-  return num;
+  int32_t min = circuit_build_times_min_timeout();
+  int32_t param = networkstatus_get_param(NULL, "cbtinitialtimeout",
+                                          CBT_DEFAULT_TIMEOUT_INITIAL_VALUE,
+                                          CBT_MIN_TIMEOUT_INITIAL_VALUE,
+                                          CBT_MAX_TIMEOUT_INITIAL_VALUE);
+  if (param < min) {
+    log_warn(LD_DIR, "Consensus parameter cbtinitialtimeout is too small, "
+             "raising to %d", min);
+    param = min;
+  }
+  return param;
 }
 
 static int32_t
diff --git a/src/or/networkstatus.c b/src/or/networkstatus.c
index 50bb88bb96..687ac03fa0 100644
--- a/src/or/networkstatus.c
+++ b/src/or/networkstatus.c
@@ -2190,15 +2190,24 @@ int32_t
 networkstatus_get_bw_weight(networkstatus_t *ns, const char *weight_name,
                             int32_t default_val)
 {
+  int32_t param;
+  int max;
   if (!ns) /* if they pass in null, go find it ourselves */
     ns = networkstatus_get_latest_consensus();
 
   if (!ns || !ns->weight_params)
     return default_val;
 
-  return get_net_param_from_list(ns->weight_params, weight_name,
-                                 default_val, -1,
-                                 circuit_build_times_get_bw_scale(ns));
+  max = circuit_build_times_get_bw_scale(ns);
+  param = get_net_param_from_list(ns->weight_params, weight_name,
+                                  default_val, -1,
+                                  BW_MAX_WEIGHT_SCALE);
+  if (param > max) {
+    log_warn(LD_DIR, "Value of consensus weight %s was too large, capping "
+             "to %d", weight_name, max);
+    param = max;
+  }
+  return param;
 }
 
 /** Return the name of the consensus flavor <b>flav</b> as used to identify
diff --git a/src/or/or.h b/src/or/or.h
index 01ff5e89d5..acca61f7d3 100644
--- a/src/or/or.h
+++ b/src/or/or.h
@@ -2981,8 +2981,8 @@ typedef uint32_t build_time_t;
  * build in terms of CDF quantile.
  */
 #define CBT_DEFAULT_CLOSE_QUANTILE 95
-/* Minimum value derived from cbtquantile parameter. */
-#define CBT_MAX_CLOSE_QUANTILE 99
+#define CBT_MIN_CLOSE_QUANTILE CBT_MIN_QUANTILE_CUTOFF
+#define CBT_MAX_CLOSE_QUANTILE CBT_MAX_QUANTILE_CUTOFF
 
 /**
  * How many circuits count as recent when considering if the
@@ -3027,9 +3027,8 @@ double circuit_build_times_quantile_cutoff(void);
 
 /** Initial circuit build timeout in milliseconds */
 #define CBT_DEFAULT_TIMEOUT_INITIAL_VALUE (60*1000)
+#define CBT_MIN_TIMEOUT_INITIAL_VALUE CBT_MIN_TIMEOUT_MIN_VALUE
 #define CBT_MAX_TIMEOUT_INITIAL_VALUE INT32_MAX
-/* CBT_MIN_TIMEOUT_INITIAL_VALUE dependent on
- * circuit_build_times_min_timeout() */
 int32_t circuit_build_times_initial_timeout(void);
 
 #if CBT_DEFAULT_MAX_RECENT_TIMEOUT_COUNT < CBT_MIN_MAX_RECENT_TIMEOUT_COUNT