Merge pull request #104 from purecloudlabs/TELECOM-11312

TELECOM-11312: Fix permission module memory access and error handling bugs

Author: benceszigeti

modules/permissions/address.c

@@ -37,6 +37,7 @@
 #include "../../parser/parse_from.h"
 #include "../../mod_fix.h"
 #include "../../resolve.h"
+#include "../../redact_pii.h"
 
 #include "permissions.h"
 #include "hash.h"
@@ -292,25 +293,27 @@ int reload_address_table(struct pm_part_struct *part_struct)
 		mask = (unsigned int) VAL_INT(val + 2);
 
 		subnet = mk_net_bitlen(ip_addr, mask);
-		if (pm_hash_insert(new_hash_table, subnet, group, port, proto,
-			&str_pattern, &str_info, mask) == -1) {
-				LM_ERR("hash table insert error\n");
-				if (subnet) {
-					pkg_free(subnet);
-				}
-				goto error;
-		}
-		if ((mask == 32 && ip_addr->af==AF_INET) || (mask == 128 && ip_addr->af==AF_INET6)) {
-			address_count += 1;
-		} else {
-			subnet_count += 1;
-		}
-		LM_DBG("Tuple <%.*s, %u, %u, %u, %.*s, %.*s> inserted into "
-				"address hash table\n", str_src_ip.len, str_src_ip.s,
-				group, port, proto, str_pattern.len, str_pattern.s,
-				str_info.len,str_info.s);
 		if (subnet) {
+			if (pm_hash_insert(new_hash_table, subnet, group, port, proto,
+				&str_pattern, &str_info, mask) == -1) {
+					LM_ERR("hash table insert error\n");
+					if (subnet) {
+						pkg_free(subnet);
+					}
+					goto error;
+			}
+			if ((mask == 32 && ip_addr->af==AF_INET) || (mask == 128 && ip_addr->af==AF_INET6)) {
+				address_count += 1;
+			} else {
+				subnet_count += 1;
+			}
+			LM_DBG("Tuple <%.*s, %u, %u, %u, %.*s, %.*s> inserted into "
+					"address hash table\n", str_src_ip.len, str_src_ip.s,
+					group, port, proto, str_pattern.len, str_pattern.s,
+					str_info.len,str_info.s);
 			pkg_free(subnet);
+		} else {
+			LM_ERR("invalid address: %.*s/%d\n", str_src_ip.len, redact_pii(str_src_ip.s), mask);
 		}
 	}
 

modules/permissions/hash.c

@@ -76,8 +76,16 @@ p_address_node_t *new_address_node(struct net *subnet, unsigned int port, int pr
                                    str *info) {
     p_address_node_t *node;
 
+    if (subnet == NULL) {
+        LM_ERR("subnet is empty\n");
+        return NULL;
+    }
+
     node = alloc_address_node(pattern, info);
-    if (!node) return NULL;
+    if (!node) {
+        LM_ERR("no shm memory left for new address node\n");
+        return NULL;
+    }
 
     node->v.port = port;
     node->v.proto = proto;
@@ -99,13 +107,13 @@ p_address_node_t *new_address_node(struct net *subnet, unsigned int port, int pr
 
 void delete_group_node(p_group_node_t *group) {
     int i;
-    p_address_node_t *address;
+    p_address_node_t *address, *next;
 
     if (!group) return;
 
     for (i = 0; i < group->v.address.bucket_count; ++i) {
-        for (address = (p_address_node_t *)group->v.address.bucket[i]; address;
-             address = address->next) {
+        for (address = (p_address_node_t *)group->v.address.bucket[i]; address; address = next) {
+            next = address->next;
             delete_address_node(address);
         }
     }
@@ -132,13 +140,15 @@ p_group_node_t *new_group_node(unsigned int group_id, unsigned int bucket_count)
     if (!node->v.ipv4_subnet) {
         LM_ERR("no shm memory left for IPv4 subnet prefix tree\n");
         shm_free(node);
+        return NULL;
     }
 
     node->v.ipv6_subnet = ppt_create_node();
     if (!node->v.ipv6_subnet) {
         LM_ERR("no shm memory left for IPv6 subnet prefix tree\n");
         ppt_free_trie(node->v.ipv4_subnet);
         shm_free(node);
+        return NULL;
     }
 
     return node;
@@ -197,10 +207,7 @@ int pm_hash_insert(p_address_table_t *table, struct net *subnet, unsigned int gr
     p_address_node_t *address;
 
     address = new_address_node(subnet, port, proto, pattern, info);
-    if (!address) {
-        LM_ERR("no shm memory left for new address node\n");
-        return -1;
-    }
+    if (!address) return -1;
 
     group = find_group_node(table, group_id);