From: Michael Rash <mbr@cipherdyne.org>
Date: Sun, 17 Jun 2012 17:42:23 +0000 (-0400)
Subject: Bug fix to throw out invalid access.conf SOURCE entries
X-Git-Tag: fwknop-2.5-pre1~275
X-Git-Url: http://www.cipherdyne.com/cgi-bin/gitweb.cgi?p=fwknop.git;a=commitdiff_plain;h=5f8e3f4a7d145670594a98802a776a26be66d577

Bug fix to throw out invalid access.conf SOURCE entries

This commit causes fwknopd to exit whenever an invalid SOURCE entry is seen
such as ":ANY".  Previous to this commit, valgrind threw the following errors
with ":ANY" as an access.conf SOURCE entry:

Invalid read of size 8
   at 0x117695: free_acc_source_list (access.c:512)
   by 0x1177E3: free_acc_stanza_data (access.c:564)
   by 0x117C67: free_acc_stanzas (access.c:654)
   by 0x10E32E: free_configs (config_init.c:106)
   by 0x10D085: main (fwknopd.c:376)
 Address 0x5a80658 is 8 bytes inside a block of size 16 free'd
   at 0x4C2A82E: free (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
   by 0x116AE0: add_source_mask (access.c:255)
   by 0x116D57: expand_acc_source (access.c:303)
   by 0x117A82: expand_acc_ent_lists (access.c:620)
   by 0x119570: parse_access_file (access.c:1043)
   by 0x10C77E: main (fwknopd.c:193)

Invalid free() / delete / delete[] / realloc()
   at 0x4C2A82E: free (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
   by 0x1176A8: free_acc_source_list (access.c:514)
   by 0x1177E3: free_acc_stanza_data (access.c:564)
   by 0x117C67: free_acc_stanzas (access.c:654)
   by 0x10E32E: free_configs (config_init.c:106)
   by 0x10D085: main (fwknopd.c:376)
 Address 0x5a80650 is 0 bytes inside a block of size 16 free'd
   at 0x4C2A82E: free (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
   by 0x116AE0: add_source_mask (access.c:255)
   by 0x116D57: expand_acc_source (access.c:303)
   by 0x117A82: expand_acc_ent_lists (access.c:620)
   by 0x119570: parse_access_file (access.c:1043)
   by 0x10C77E: main (fwknopd.c:193)

HEAP SUMMARY:
    in use at exit: 8 bytes in 1 blocks
  total heap usage: 1,659 allocs, 1,659 frees, 238,310 bytes allocated
---

diff --git a/server/access.c b/server/access.c
index 331a805..67ceb3e 100644
--- a/server/access.c
+++ b/server/access.c
@@ -187,7 +187,7 @@ add_acc_force_nat(fko_srv_options_t *opts, acc_stanza_t *curr_acc, const char *v
  * comparisons of incoming source IPs against this mask.
 */
 static void
-add_source_mask(acc_stanza_t *acc, const char *ip)
+add_source_mask(fko_srv_options_t *opts, acc_stanza_t *acc, const char *ip)
 {
     char                *ndx;
     char                ip_str[MAX_IPV4_STR_LEN] = {0};
@@ -202,7 +202,7 @@ add_source_mask(acc_stanza_t *acc, const char *ip)
         log_msg(LOG_ERR,
             "Fatal memory allocation error adding stanza source_list entry"
         );
-        exit(EXIT_FAILURE);
+        clean_exit(opts, NO_FW_CLEANUP, EXIT_FAILURE);
     }
 
     /* If this is not the first entry, we walk our pointer to the
@@ -249,13 +249,9 @@ add_source_mask(acc_stanza_t *acc, const char *ip)
         if(inet_aton(ip_str, &in) == 0)
         {
             log_msg(LOG_ERR,
-                "Error parsing IP to int for: %s", ip_str
+                "Fatal error parsing IP to int for: %s", ip_str
             );
-
-            free(new_sle);
-            new_sle = NULL;
-
-            return;
+            clean_exit(opts, NO_FW_CLEANUP, EXIT_FAILURE);
         }
 
         /* Store our mask converted from CIDR to a 32-bit value.
@@ -272,7 +268,7 @@ add_source_mask(acc_stanza_t *acc, const char *ip)
 /* Expand the access SOURCE string to a list of masks.
 */
 void
-expand_acc_source(acc_stanza_t *acc)
+expand_acc_source(fko_srv_options_t *opts, acc_stanza_t *acc)
 {
     char           *ndx, *start;
     char            buf[32];
@@ -289,7 +285,7 @@ expand_acc_source(acc_stanza_t *acc)
                 start++;
 
             strlcpy(buf, start, (ndx-start)+1);
-            add_source_mask(acc, buf);
+            add_source_mask(opts, acc, buf);
             start = ndx+1;
         }
     }
@@ -300,7 +296,7 @@ expand_acc_source(acc_stanza_t *acc)
         start++;
 
     strlcpy(buf, start, (ndx-start)+1);
-    add_source_mask(acc, buf);
+    add_source_mask(opts, acc, buf);
 }
 
 static int
@@ -617,7 +613,7 @@ expand_acc_ent_lists(fko_srv_options_t *opts)
     {
         /* Expand the source string to 32-bit integer masks foreach entry.
         */
-        expand_acc_source(acc);
+        expand_acc_source(opts, acc);
 
         /* Now expand the open_ports string.
         */