Fix an overflow when counting rejects for *, truncate exit summaries after 1000 chars

svn:r16530

doc/spec/proposals/141-jit-sd-downloads.txt

@@ -246,6 +246,13 @@ Status: Draft
   of "80-88,89-100" there only is a single item of "80-100", similarly
   instead of "20,21" a summary will say "20-21".
 
+  Port lists are sorted in ascending order.
+
+  The maximum allowed length of a policy summary (including the "accept "
+  or "reject ") is 1000 characters.  If a summary exceeds that length we
+  use an accept-style summary and list as much of the port list as is
+  possible within these 1000 bytes.
+
   Similarly to IP address, ports, and timestamp a consensus should list
   the exit policy matching the descriptor digest referenced in the
   consensus document (See dir-spec section 3.4).

src/or/or.h

@@ -3208,11 +3208,19 @@ download_status_is_ready(download_status_t *dls, time_t now,
 }
 
 /********************************* dirserv.c ***************************/
+/** Maximum length of an exit policy summary. */
+#define MAX_EXITPOLICY_SUMMARY_LEN (1000)
+
 /** Maximum allowable length of a version line in a networkstatus. */
 #define MAX_V_LINE_LEN 128
 /** Length of "r Authority BadDirectory BadExit Exit Fast Guard HSDir Named
  * Running Stable Unnamed V2Dir Valid\n". */
 #define MAX_FLAG_LINE_LEN 96
+/** Length of "w" line for weighting.  Currently at most
+ * "w Bandwidth=<uint32t>\n" */
+#define MAX_WEIGHT_LINE_LEN (13+10)
+/** Maximum length of an exit policy summary line. */
+#define MAX_POLICY_LINE_LEN (3+MAX_EXITPOLICY_SUMMARY_LEN)
 /** Amount of space to allocate for each entry: r, s, and v lines. */
 #define RS_ENTRY_LEN                                                    \
   ( /* first line */                                                    \
@@ -3220,6 +3228,10 @@ download_status_is_ready(download_status_t *dls, time_t now,
    5*2 /* ports */ + 10 /* punctuation */ +                             \
    /* second line */                                                    \
    MAX_FLAG_LINE_LEN +                                                  \
+   /* weight line */                                                    \
+   MAX_WEIGHT_LINE_LEN +                                                \
+   /* p line. */                                                        \
+   MAX_POLICY_LINE_LEN +                                                \
    /* v line. */                                                        \
    MAX_V_LINE_LEN                                                       \
    )
@@ -4108,6 +4120,7 @@ routerinfo_t *router_find_exact_exit_enclave(const char *address,
 int router_is_unreliable(routerinfo_t *router, int need_uptime,
                          int need_capacity, int need_guard);
 uint32_t router_get_advertised_bandwidth(routerinfo_t *router);
+uint32_t router_get_advertised_bandwidth_capped(routerinfo_t *router);
 
 typedef enum {
   NO_WEIGHTING, WEIGHT_FOR_EXIT, WEIGHT_FOR_GUARD

src/or/policies.c

@@ -1039,7 +1039,7 @@ policy_summary_reject(smartlist_t *summary,
 {
   int i = policy_summary_split(summary, prt_min, prt_max);
   /* XXX: ipv4 specific */
-  int count = (1 << (32-maskbits));
+  uint64_t count = (U64_LITERAL(1) << (32-maskbits));
   while (i < smartlist_len(summary) &&
          AT(i)->prt_max <= prt_max) {
     AT(i)->reject_count += count;
@@ -1160,7 +1160,21 @@ policy_summarize(smartlist_t *policy)
   accepts_str = smartlist_join_strings(accepts, ",", 0, &accepts_len);
   rejects_str = smartlist_join_strings(rejects, ",", 0, &rejects_len);
 
-  if (rejects_len < accepts_len) {
+  if (rejects_len > MAX_EXITPOLICY_SUMMARY_LEN &&
+      accepts_len > MAX_EXITPOLICY_SUMMARY_LEN) {
+    char *c;
+    shorter_str = accepts_str;
+    prefix = "accept";
+
+    c = shorter_str + (MAX_EXITPOLICY_SUMMARY_LEN-strlen(prefix)-1);
+    while (*c != ',' && c >= shorter_str)
+      c--;
+    tor_assert(c >= shorter_str);
+    tor_assert(*c == ',');
+    *c = '\0';
+
+    shorter_len = strlen(shorter_str);
+  } else if (rejects_len < accepts_len) {
     shorter_str = rejects_str;
     shorter_len = rejects_len;
     prefix = "reject";
@@ -1171,6 +1185,7 @@ policy_summarize(smartlist_t *policy)
   }
 
   final_size = strlen(prefix)+1+shorter_len+1;
+  tor_assert(final_size <= MAX_EXITPOLICY_SUMMARY_LEN+1);
   result = malloc(final_size);
   tor_snprintf(result, final_size, "%s %s", prefix, shorter_str);
 

src/or/test.c

@@ -3355,10 +3355,13 @@ test_policy_summary_helper(const char *policy_str,
 static void
 test_policies(void)
 {
+  int i;
   smartlist_t *policy, *policy2;
   addr_policy_t *p;
   tor_addr_t tar;
   config_line_t line;
+  smartlist_t *sm;
+  char *policy_str;
 
   policy = smartlist_create();
 
@@ -3458,6 +3461,46 @@ test_policies(void)
                              "accept *:65535,"
                              "reject *:*",
                              "accept 1,3,65535");
+  /* holes */
+  test_policy_summary_helper("accept *:1,"
+                             "accept *:3,"
+                             "accept *:5,"
+                             "accept *:7,"
+                             "reject *:*",
+                             "accept 1,3,5,7");
+  test_policy_summary_helper("reject *:1,"
+                             "reject *:3,"
+                             "reject *:5,"
+                             "reject *:7,"
+                             "accept *:*",
+                             "reject 1,3,5,7");
+  /* truncation ports */
+  sm = smartlist_create();
+  for (i=1; i<2000; i+=2) {
+    char buf[POLICY_BUF_LEN];
+    tor_snprintf(buf, sizeof(buf), "reject *:%d", i);
+    smartlist_add(sm, tor_strdup(buf));
+  }
+  smartlist_add(sm, tor_strdup("accept *:*"));
+  policy_str = smartlist_join_strings(sm, ",", 0, NULL);
+  test_policy_summary_helper( policy_str,
+    "accept 2,4,6,8,10,12,14,16,18,20,22,24,26,28,30,32,34,36,38,40,42,44,"
+    "46,48,50,52,54,56,58,60,62,64,66,68,70,72,74,76,78,80,82,84,86,88,90,"
+    "92,94,96,98,100,102,104,106,108,110,112,114,116,118,120,122,124,126,128,"
+    "130,132,134,136,138,140,142,144,146,148,150,152,154,156,158,160,162,164,"
+    "166,168,170,172,174,176,178,180,182,184,186,188,190,192,194,196,198,200,"
+    "202,204,206,208,210,212,214,216,218,220,222,224,226,228,230,232,234,236,"
+    "238,240,242,244,246,248,250,252,254,256,258,260,262,264,266,268,270,272,"
+    "274,276,278,280,282,284,286,288,290,292,294,296,298,300,302,304,306,308,"
+    "310,312,314,316,318,320,322,324,326,328,330,332,334,336,338,340,342,344,"
+    "346,348,350,352,354,356,358,360,362,364,366,368,370,372,374,376,378,380,"
+    "382,384,386,388,390,392,394,396,398,400,402,404,406,408,410,412,414,416,"
+    "418,420,422,424,426,428,430,432,434,436,438,440,442,444,446,448,450,452,"
+    "454,456,458,460,462,464,466,468,470,472,474,476,478,480,482,484,486,488,"
+    "490,492,494,496,498,500,502,504,506,508,510,512,514,516,518,520,522");
+  tor_free(policy_str);
+  SMARTLIST_FOREACH(sm, char *, s, tor_free(s));
+  smartlist_clear(sm);
 }
 
 static void