[tor-commits] [tor/master] Fix some leaks/missed checks in the unit tests

nickm at torproject.org nickm at torproject.org
Thu Mar 13 14:17:10 UTC 2014


commit 119896cd43f420a053c552afe390f6d66224b3b7
Author: Nick Mathewson <nickm at torproject.org>
Date:   Thu Mar 13 10:07:10 2014 -0400

    Fix some leaks/missed checks in the unit tests
    
    Coverity spotted these.
---
 ChangeLog                  |   22 +++++++++++-----------
 src/common/sandbox.c       |    2 +-
 src/test/test.c            |   38 ++++++++++++++++++++++----------------
 src/test/test_routerkeys.c |    2 +-
 4 files changed, 35 insertions(+), 29 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 8002e4e..3968337 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,9 +1,19 @@
 Changes in version 0.2.5.3-alpha - 2014-03-??
 
-  o Major features:
+  o Major features (security, DoS-resistance):
     - Also consider stream buffer sizes when calculating OOM
       conditions. Rename MaxMemInCellQueues to MaxMemInQueues. Fixes
       bug 10169.
+    - Avoid hash-flooding denial-of-service attacks by using the secure
+      SipHash-2-4 hash function for our hashtables.  Without this
+      feature, an attacker could degrade performance of a targeted
+      client or server by flooding their data structures with a large
+      number of data entries all calculated to be stored at the same
+      hash table position, thereby degrading hash table
+      performance. With this feature, hash table positions are derived
+      from a randomized cryptographic key using SipHash-2-4, and an
+      attacker cannot predict which entries will collide.
+      Closes ticket 4900.
 
   o Minor features:
     - Bridges write the SHA1 digest of their identity key fingerprint to
@@ -23,16 +33,6 @@ Changes in version 0.2.5.3-alpha - 2014-03-??
       database.
     - Decrease the lower limit of MaxMemInQueues to 256 MBytes, to
       appease raspberry pi users. Fixes bug 9686.
-    - Avoid hash-flooding denial-of-service attacks by using the secure
-      SipHash-2-4 hash function for our hashtables.  Without this
-      feature, an attacker could degrade performance of a targeted
-      client or server by flooding their data structures with a large
-      number of data entries all calculated to be stored at the same
-      hash table position, thereby degrading hash table
-      performance. With this feature, hash table positions are derived
-      from a randomized cryptographic key using SipHash-2-4, and an
-      attacker cannot predict which entries will collide.
-      Closes ticket 4900.
     - Made PREDICTED_CIRCS_RELEVANCE_TIME configurable from config
       file with a new option, PredictedPortsRelevanceTime. Implements
       ticket #9176. Patch by unixninja92.
diff --git a/src/common/sandbox.c b/src/common/sandbox.c
index 6b78748..5775289 100644
--- a/src/common/sandbox.c
+++ b/src/common/sandbox.c
@@ -1326,7 +1326,7 @@ sigsys_debugging(int nr, siginfo_t *info, void *void_context)
   if (!ctx)
     return;
 
-  syscall = ctx->uc_mcontext.gregs[REG_SYSCALL];
+  syscall = (int) ctx->uc_mcontext.gregs[REG_SYSCALL];
 
   format_dec_number_sigsafe(syscall, number, sizeof(number));
   tor_log_err_sigsafe("(Sandbox) Caught a bad syscall attempt (syscall ",
diff --git a/src/test/test.c b/src/test/test.c
index 456dde1..0ba5da3 100644
--- a/src/test/test.c
+++ b/src/test/test.c
@@ -671,6 +671,7 @@ test_policies(void)
   config_line_t line;
   smartlist_t *sm = NULL;
   char *policy_str = NULL;
+  short_policy_t *short_parsed = NULL;
 
   policy = smartlist_new();
 
@@ -858,24 +859,28 @@ test_policies(void)
   test_short_policy_parse("reject ,1-10,,,,30-40", "reject 1-10,30-40");
 
   /* Try parsing various broken short policies */
-  tt_ptr_op(NULL, ==, parse_short_policy("accept 200-199"));
-  tt_ptr_op(NULL, ==, parse_short_policy(""));
-  tt_ptr_op(NULL, ==, parse_short_policy("rejekt 1,2,3"));
-  tt_ptr_op(NULL, ==, parse_short_policy("reject "));
-  tt_ptr_op(NULL, ==, parse_short_policy("reject"));
-  tt_ptr_op(NULL, ==, parse_short_policy("rej"));
-  tt_ptr_op(NULL, ==, parse_short_policy("accept 2,3,100000"));
-  tt_ptr_op(NULL, ==, parse_short_policy("accept 2,3x,4"));
-  tt_ptr_op(NULL, ==, parse_short_policy("accept 2,3x,4"));
-  tt_ptr_op(NULL, ==, parse_short_policy("accept 2-"));
-  tt_ptr_op(NULL, ==, parse_short_policy("accept 2-x"));
-  tt_ptr_op(NULL, ==, parse_short_policy("accept 1-,3"));
-  tt_ptr_op(NULL, ==, parse_short_policy("accept 1-,3"));
+#define TT_BAD_SHORT_POLICY(s)                                          \
+  do {                                                                  \
+    tt_ptr_op(NULL, ==, (short_parsed = parse_short_policy((s))));      \
+  } while (0)
+  TT_BAD_SHORT_POLICY("accept 200-199");
+  TT_BAD_SHORT_POLICY("");
+  TT_BAD_SHORT_POLICY("rejekt 1,2,3");
+  TT_BAD_SHORT_POLICY("reject ");
+  TT_BAD_SHORT_POLICY("reject");
+  TT_BAD_SHORT_POLICY("rej");
+  TT_BAD_SHORT_POLICY("accept 2,3,100000");
+  TT_BAD_SHORT_POLICY("accept 2,3x,4");
+  TT_BAD_SHORT_POLICY("accept 2,3x,4");
+  TT_BAD_SHORT_POLICY("accept 2-");
+  TT_BAD_SHORT_POLICY("accept 2-x");
+  TT_BAD_SHORT_POLICY("accept 1-,3");
+  TT_BAD_SHORT_POLICY("accept 1-,3");
+
   /* Test a too-long policy. */
   {
     int i;
     char *policy = NULL;
-    short_policy_t *parsed;
     smartlist_t *chunks = smartlist_new();
     smartlist_add(chunks, tor_strdup("accept "));
     for (i=1; i<10000; ++i)
@@ -884,9 +889,9 @@ test_policies(void)
     policy = smartlist_join_strings(chunks, "", 0, NULL);
     SMARTLIST_FOREACH(chunks, char *, ch, tor_free(ch));
     smartlist_free(chunks);
-    parsed = parse_short_policy(policy);/* shouldn't be accepted */
+    short_parsed = parse_short_policy(policy);/* shouldn't be accepted */
     tor_free(policy);
-    tt_ptr_op(NULL, ==, parsed);
+    tt_ptr_op(NULL, ==, short_parsed);
   }
 
   /* truncation ports */
@@ -927,6 +932,7 @@ test_policies(void)
     SMARTLIST_FOREACH(sm, char *, s, tor_free(s));
     smartlist_free(sm);
   }
+  short_policy_free(short_parsed);
 }
 
 /** Test encoding and parsing of rendezvous service descriptors. */
diff --git a/src/test/test_routerkeys.c b/src/test/test_routerkeys.c
index ff52a7e..1c8174b 100644
--- a/src/test/test_routerkeys.c
+++ b/src/test/test_routerkeys.c
@@ -32,7 +32,7 @@ test_routerkeys_write_fingerprint(void *arg)
   set_server_identity_key(key);
   set_client_identity_key(crypto_pk_dup_key(key));
 
-  check_private_dir(ddir, CPD_CREATE, NULL);
+  tt_int_op(0, ==, check_private_dir(ddir, CPD_CREATE, NULL));
   tt_int_op(crypto_pk_cmp_keys(get_server_identity_key(),key),==,0);
 
   /* Write fingerprint file */



More information about the tor-commits mailing list