From c65e25c6568c53d44d0163ebd4889260466bcdfa Mon Sep 17 00:00:00 2001
From: Michael Rash <mbr@cipherdyne.org>
Date: Thu, 8 Sep 2011 21:33:52 -0400
Subject: [PATCH] Check for active_rules > 0 before decrementing

In the fw_config struct the active_rules member is unsigned, so this change
ensures that we don't try to decrement it below zero whenever a firewall rule
is deleted or an error condition occurs.
---
 server/fw_util_ipfw.c     | 25 ++++++++++++++++++-------
 server/fw_util_iptables.c | 23 ++++++++++++-----------
 2 files changed, 30 insertions(+), 18 deletions(-)

diff --git a/server/fw_util_ipfw.c b/server/fw_util_ipfw.c
index 5434a9ed..3a3d9d56 100644
--- a/server/fw_util_ipfw.c
+++ b/server/fw_util_ipfw.c
@@ -489,8 +489,6 @@ check_firewall_rules(fko_srv_options_t *opts)
     time_t          now, rule_exp, min_exp = 0;
     unsigned short  curr_rule;
 
-    time(&now);
-
     /* Just in case we somehow lose track and fall out-of-whack.
     */
     if(fwc.active_rules > fwc.max_rules)
@@ -499,7 +497,12 @@ check_firewall_rules(fko_srv_options_t *opts)
     /* If there are no active rules or we have not yet
      * reached our expected next expire time, continue.
     */
-    if(fwc.active_rules == 0 || fwc.next_expire > now)
+    if(fwc.active_rules == 0)
+        return;
+
+    time(&now);
+
+    if (fwc.next_expire > now)
         return;
 
     zero_cmd_buffers();
@@ -534,7 +537,9 @@ check_firewall_rules(fko_srv_options_t *opts)
         log_msg(LOG_ERR,
             "Did not find expire comment in rules list %i.\n", i);
 
-        fwc.active_rules--;
+        if (fwc.active_rules > 0)
+            fwc.active_rules--;
+
         return;
     }
 
@@ -577,7 +582,9 @@ check_firewall_rules(fko_srv_options_t *opts)
                 log_msg(LOG_ERR,
                     "Rule parse error while finding rule line start.");
 
-                fwc.active_rules--;
+                if (fwc.active_rules > 0)
+                    fwc.active_rules--;
+
                 break;
             }
 
@@ -591,7 +598,9 @@ check_firewall_rules(fko_srv_options_t *opts)
                 log_msg(LOG_ERR,
                     "Rule parse error while finding rule number.");
 
-                fwc.active_rules--;
+                if (fwc.active_rules > 0)
+                    fwc.active_rules--;
+
                 break;
             }
              
@@ -617,7 +626,9 @@ check_firewall_rules(fko_srv_options_t *opts)
                     rule_num_str, rule_exp, fwc.expire_set_num
                 );
 
-                fwc.active_rules--;
+                if (fwc.active_rules > 0)
+                    fwc.active_rules--;
+
                 fwc.rule_map[curr_rule - fwc.start_rule_num] = RULE_EXPIRED;
             }
             else
diff --git a/server/fw_util_iptables.c b/server/fw_util_iptables.c
index 91a791c0..6d62a7e5 100644
--- a/server/fw_util_iptables.c
+++ b/server/fw_util_iptables.c
@@ -761,13 +761,6 @@ check_firewall_rules(fko_srv_options_t *opts)
     */
     for(i = 0; i < NUM_FWKNOP_ACCESS_TYPES; i++)
     {
-        /* Just in case we somehow lose track and fall out-of-whack,
-         * we be the hero and reset it to zero.
-         *  (poet but don't know it :-o )
-        */
-        if(ch[i].active_rules < 0)
-            ch[i].active_rules = 0;
-
         /* If there are no active rules or we have not yet
          * reached our expected next expire time, continue.
         */
@@ -806,7 +799,9 @@ check_firewall_rules(fko_srv_options_t *opts)
             log_msg(LOG_ERR,
                 "Did not find expire comment in rules list %i.\n", i);
 
-            ch[i].active_rules--;
+            if (ch[i].active_rules > 0)
+                ch[i].active_rules--;
+
             continue;
         }
 
@@ -845,7 +840,9 @@ check_firewall_rules(fko_srv_options_t *opts)
                     log_msg(LOG_ERR,
                         "Rule parse error while finding rule line start in chain %i", i);
 
-                    ch[i].active_rules--;
+                    if (ch[i].active_rules > 0)
+                        ch[i].active_rules--;
+
                     break;
                 }
                 rn_start++;
@@ -859,7 +856,9 @@ check_firewall_rules(fko_srv_options_t *opts)
                     log_msg(LOG_ERR,
                         "Rule parse error while finding rule number in chain %i", i);
 
-                    ch[i].active_rules--;
+                    if (ch[i].active_rules > 0)
+                        ch[i].active_rules--;
+
                     break;
                 }
                  
@@ -884,7 +883,9 @@ check_firewall_rules(fko_srv_options_t *opts)
                     );
 
                     rn_offset++;
-                    ch[i].active_rules--;
+
+                    if (ch[i].active_rules > 0)
+                        ch[i].active_rules--;
                 }
                 else
                     log_msg(LOG_ERR, "Error %i from cmd:'%s': %s", res, cmd_buf, err_buf);