Fix parse_short_policy (bug 7192.)

Our implementation of parse_short_policy was screwed up: it would
ignore the last character of every short policy.  Obviously, that's
broken.

This patch fixes the busted behavior, and adds a bunch of unit tests
to make sure the rest of that function is okay.

Fixes bug 7192; fix on 0.2.3.1-alpha.

1 files changed, 11 insertions, 7 deletions

diff --git a/src/or/policies.c b/src/or/policies.c
index 486c2647e..bbd6816ff 100644
--- a/src/or/policies.c
+++ b/src/or/policies.c
@@ -1348,8 +1348,10 @@ parse_short_policy(const char *summary)
     unsigned low, high;
     char dummy;
     char ent_buf[32];
+    size_t len;
 
     next = comma ? comma+1 : strchr(summary, '\0');
+    len = comma ? (size_t)(comma - summary) : strlen(summary);
 
     if (n_entries == MAX_EXITPOLICY_SUMMARY_LEN) {
       log_fn(LOG_PROTOCOL_WARN, LD_DIR, "Impossibly long policy summary %s",
@@ -1357,20 +1359,22 @@ parse_short_policy(const char *summary)
       return NULL;
     }
 
-    if (! TOR_ISDIGIT(*summary) || next-summary > (int)(sizeof(ent_buf)-1)) {
+    if (! TOR_ISDIGIT(*summary) || len > (sizeof(ent_buf)-1)) {
       /* unrecognized entry format. skip it. */
       continue;
     }
-    if (next-summary < 2) {
+    if (len < 1) {
       /* empty; skip it. */
+      /* XXX This happens to be unreachable, since if len==0, then *summary is
+       * ',' or '\0', and the TOR_ISDIGIT test above would have failed. */
       continue;
     }
 
-    memcpy(ent_buf, summary, next-summary-1);
-    ent_buf[next-summary-1] = '\0';
+    memcpy(ent_buf, summary, len);
+    ent_buf[len] = '\0';
 
     if (tor_sscanf(ent_buf, "%u-%u%c", &low, &high, &dummy) == 2) {
-      if (low<1 || low>65535 || high<1 || high>65535) {
+      if (low<1 || low>65535 || high<1 || high>65535 || low>high) {
         log_fn(LOG_PROTOCOL_WARN, LD_DIR,
                "Found bad entry in policy summary %s", escaped(orig_summary));
         return NULL;
@@ -1415,7 +1419,7 @@ parse_short_policy(const char *summary)
 
 /** Write <b>policy</b> back out into a string. Used only for unit tests
  * currently. */
-const char *
+char *
 write_short_policy(const short_policy_t *policy)
 {
   int i;
@@ -1424,7 +1428,7 @@ write_short_policy(const short_policy_t *policy)
 
   smartlist_add_asprintf(sl, "%s", policy->is_accept ? "accept " : "reject ");
 
-  for(i=0; i < policy->n_entries; i++) {
+  for (i=0; i < policy->n_entries; i++) {
     const short_policy_entry_t *e = &policy->entries[i];
     if (e->min_port == e->max_port) {
       smartlist_add_asprintf(sl, "%d", e->min_port);