[or-cvs] r14078: Backport: Fix policy-related crash bug found by lodger. (in tor/branches/tor-0_2_0-patches: . src/or)

[nickm@xxxxxxxx]

Author: nickm
Date: 2008-03-17 12:52:01 -0400 (Mon, 17 Mar 2008)
New Revision: 14078

Modified:
   tor/branches/tor-0_2_0-patches/
   tor/branches/tor-0_2_0-patches/ChangeLog
   tor/branches/tor-0_2_0-patches/src/or/or.h
   tor/branches/tor-0_2_0-patches/src/or/policies.c
   tor/branches/tor-0_2_0-patches/src/or/test.c
Log:
 r18881@catbus:  nickm | 2008-03-17 12:51:33 -0400
 Backport: Fix policy-related crash bug found by lodger.



Property changes on: tor/branches/tor-0_2_0-patches
___________________________________________________________________
 svk:merge ticket from /tor/020 [r18881] on 8246c3cf-6607-4228-993b-4d95d33730f1

Modified: tor/branches/tor-0_2_0-patches/ChangeLog
===================================================================
--- tor/branches/tor-0_2_0-patches/ChangeLog	2008-03-17 16:51:48 UTC (rev 14077)
+++ tor/branches/tor-0_2_0-patches/ChangeLog	2008-03-17 16:52:01 UTC (rev 14078)
@@ -38,6 +38,9 @@
     - Avoid double-marked-for-close warning when certain kinds of invalid
       .in-addr.arpa addresses are passed to the DNSPort.  Part of a fix
       for bug 617. Bugfix on 0.2.0.1-alpha.
+    - 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:
     - Only log guard node status when guard node status has changed.

Modified: tor/branches/tor-0_2_0-patches/src/or/or.h
===================================================================
--- tor/branches/tor-0_2_0-patches/src/or/or.h	2008-03-17 16:51:48 UTC (rev 14077)
+++ tor/branches/tor-0_2_0-patches/src/or/or.h	2008-03-17 16:52:01 UTC (rev 14078)
@@ -1118,7 +1118,7 @@
   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.*/

Modified: tor/branches/tor-0_2_0-patches/src/or/policies.c
===================================================================
--- tor/branches/tor-0_2_0-patches/src/or/policies.c	2008-03-17 16:51:48 UTC (rev 14077)
+++ tor/branches/tor-0_2_0-patches/src/or/policies.c	2008-03-17 16:52:01 UTC (rev 14078)
@@ -530,8 +530,11 @@
   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);
     maybe = 0;
@@ -764,6 +767,9 @@
   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 @@
 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;

Modified: tor/branches/tor-0_2_0-patches/src/or/test.c
===================================================================
--- tor/branches/tor-0_2_0-patches/src/or/test.c	2008-03-17 16:51:48 UTC (rev 14077)
+++ tor/branches/tor-0_2_0-patches/src/or/test.c	2008-03-17 16:52:01 UTC (rev 14078)
@@ -2996,6 +2996,8 @@
           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));
@@ -3003,12 +3005,16 @@
 
   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);