From 9a24158c698bd2cce6ddbea365a15f7e02bddf3a Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Mon, 17 Mar 2008 16:51:48 +0000 Subject: [PATCH] r18880@catbus: nickm | 2008-03-17 12:51:24 -0400 Fix policy-related crash bug found by lodger. svn:r14077 --- ChangeLog | 3 +++ src/or/or.h | 2 +- src/or/policies.c | 10 +++++++++- src/or/test.c | 6 ++++++ 4 files changed, 19 insertions(+), 2 deletions(-) diff --git a/ChangeLog b/ChangeLog index 7e31e99d78..4a04efa1b3 100644 --- a/ChangeLog +++ b/ChangeLog @@ -29,6 +29,9 @@ Changes in version 0.2.1.1-alpha - 2008-??-?? - Detect mismatched page sizes when using --enable-openbsd-malloc. Bugfix on 0.2.0.x. - Stop giving double-close warn when we reject an address for client DNS. + - Make sure that the "NULL-means-reject *:*" convention is followed by + all the policy manipulation functions, avoiding some possible crash + bugs. Bug found by lodger. Bugfix on 0.2.0.16-alpha. o Minor features: - Allow separate log levels to be configured for different logging diff --git a/src/or/or.h b/src/or/or.h index 1830889d54..6a98a34209 100644 --- a/src/or/or.h +++ b/src/or/or.h @@ -1118,7 +1118,7 @@ typedef enum { ADDR_POLICY_REJECT=2, } addr_policy_action_t; -/** A linked list of policy rules */ +/** A reference-counted address policy rule. */ typedef struct addr_policy_t { int refcnt; /**< Reference count */ addr_policy_action_t policy_type:2;/**< What to do when the policy matches.*/ diff --git a/src/or/policies.c b/src/or/policies.c index 882540edca..20c1fb9186 100644 --- a/src/or/policies.c +++ b/src/or/policies.c @@ -530,7 +530,10 @@ compare_addr_to_addr_policy(uint32_t addr, uint16_t port, int match = 0; int maybe = 0; int i, len; - len = policy ? smartlist_len(policy) : 0; + if (!policy) + return ADDR_POLICY_REJECTED; + + len = smartlist_len(policy); for (i = 0; i < len; ++i) { addr_policy_t *tmpe = smartlist_get(policy, i); @@ -764,6 +767,9 @@ exit_policy_is_general_exit(smartlist_t *policy) static const int ports[] = { 80, 443, 6667 }; int n_allowed = 0; int i; + if (!policy) + return 0; + for (i = 0; i < 3; ++i) { SMARTLIST_FOREACH(policy, addr_policy_t *, p, { if (p->prt_min > ports[i] || p->prt_max < ports[i]) @@ -787,6 +793,8 @@ exit_policy_is_general_exit(smartlist_t *policy) int policy_is_reject_star(smartlist_t *policy) { + if (!policy) + return 1; SMARTLIST_FOREACH(policy, addr_policy_t *, p, { if (p->policy_type == ADDR_POLICY_ACCEPT) return 0; diff --git a/src/or/test.c b/src/or/test.c index 32e22cb754..db6e96f7c2 100644 --- a/src/or/test.c +++ b/src/or/test.c @@ -3034,6 +3034,8 @@ test_policies(void) compare_addr_to_addr_policy(0, 2, policy)); test_assert(ADDR_POLICY_REJECTED == compare_addr_to_addr_policy(0xc0a80102, 2, policy)); + test_assert(ADDR_POLICY_REJECTED == + compare_addr_to_addr_policy(0x01020304u, 2, NULL)); policy2 = NULL; test_assert(0 == policies_parse_exit_policy(NULL, &policy2, 1, NULL)); @@ -3041,12 +3043,16 @@ test_policies(void) test_assert(!exit_policy_is_general_exit(policy)); test_assert(exit_policy_is_general_exit(policy2)); + test_assert(!exit_policy_is_general_exit(NULL)); test_assert(cmp_addr_policies(policy, policy2)); + test_assert(cmp_addr_policies(policy, NULL)); test_assert(!cmp_addr_policies(policy2, policy2)); + test_assert(!cmp_addr_policies(NULL, NULL)); test_assert(!policy_is_reject_star(policy2)); test_assert(policy_is_reject_star(policy)); + test_assert(policy_is_reject_star(NULL)); addr_policy_list_free(policy); addr_policy_list_free(policy2);