From 119896cd43f420a053c552afe390f6d66224b3b7 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Thu, 13 Mar 2014 10:07:10 -0400 Subject: 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 8002e4e04..3968337ec 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 6b7874883..577528988 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 456dde141..0ba5da367 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 ff52a7e7c..1c8174b06 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 */ -- cgit v1.2.3