From 5d92e448ae937addd89f9de57f2366355047643b Mon Sep 17 00:00:00 2001 From: Felipe Zimmerle Date: Tue, 10 Jun 2014 16:09:05 -0700 Subject: [PATCH] Fixes subnets representations using slash notation The ipv4 representation was only accepting slash notation with masks represented in 2 digits. In the ipv6 implementation several fixies were made: The maximum value to a bitmask was 64 which is not the reality, as ipv6 can handle 128 bits. The second change was also to enable mask representation with more and less than 2 digits. A more general fix was added to allow the unit tests to work even if a invalid ip/range was informed during the creation of the "tree", now it is checking if the tree is NULL while performing the execution of the operator. Initial problem was reported at the issue: #706. --- apache2/msc_tree.c | 39 ++++++++++++++++++++++++++++++--------- apache2/msc_util.c | 4 ++-- apache2/re_operators.c | 6 ++++-- 3 files changed, 36 insertions(+), 13 deletions(-) diff --git a/apache2/msc_tree.c b/apache2/msc_tree.c index 127ae24a..c672bd5f 100644 --- a/apache2/msc_tree.c +++ b/apache2/msc_tree.c @@ -820,35 +820,46 @@ TreeNode *TreeAddIP(const char *buffer, CPTTree *tree, int type) { char ip_strv4[NETMASK_32], ip_strv6[NETMASK_128]; struct in_addr addr4; struct in6_addr addr6; + int pos = 0; char *ptr = NULL; if(tree == NULL) return NULL; + pos = strchr(buffer, '/') - buffer; + switch(type) { case IPV4_TREE: memset(&addr4, 0, sizeof(addr4)); memset(ip_strv4, 0x0, NETMASK_32); - strncpy(ip_strv4, buffer, sizeof(ip_strv4) - 2); + strncpy(ip_strv4, buffer, sizeof(ip_strv4)); *(ip_strv4 + (sizeof(ip_strv4) - 1)) = '\0'; ptr = strdup(ip_strv4); netmask_v4 = is_netmask_v4(ptr); + if (netmask_v4 > NETMASK_32) { + free(ptr); + ptr = NULL; + return NULL; + } + if(ptr != NULL) { free(ptr); ptr = NULL; } - if(netmask_v4 == 0) + if(netmask_v4 == 0) { return NULL; - else if(netmask_v4 != NETMASK_32) { - ip_strv4[strlen(ip_strv4)-3] = '\0'; + } + else if (netmask_v4 != NETMASK_32 && pos < strlen(ip_strv4)) { + ip_strv4[pos] = '\0'; } ret = inet_pton(AF_INET, ip_strv4, &addr4); + if (ret <= 0) { return NULL; } @@ -863,26 +874,36 @@ TreeNode *TreeAddIP(const char *buffer, CPTTree *tree, int type) { memset(&addr6, 0, sizeof(addr6)); memset(ip_strv6, 0x0, NETMASK_128); - strncpy(ip_strv6, buffer, sizeof(ip_strv6) - 2); + strncpy(ip_strv6, buffer, sizeof(ip_strv6)); *(ip_strv6 + sizeof(ip_strv6) - 1) = '\0'; ptr = strdup(ip_strv6); netmask_v6 = is_netmask_v6(ptr); + if (netmask_v6 > NETMASK_128) { + free(ptr); + ptr = NULL; + return NULL; + } + if(ptr != NULL) { free(ptr); ptr = NULL; } - if(netmask_v6 == 0) + if(netmask_v6 == 0) { return NULL; - else if (netmask_v6 != NETMASK_64) { - ip_strv4[strlen(ip_strv6)-3] = '\0'; + } + else if (netmask_v6 != NETMASK_128 && pos < strlen(ip_strv6)) { + ip_strv6[pos] = '\0'; } ret = inet_pton(AF_INET6, ip_strv6, &addr6); - if(ret <= 0) + + if (ret <= 0) + { return NULL; + } tree->count++; diff --git a/apache2/msc_util.c b/apache2/msc_util.c index 3aa619fb..885b378b 100644 --- a/apache2/msc_util.c +++ b/apache2/msc_util.c @@ -364,12 +364,12 @@ unsigned char is_netmask_v6(char *ip_strv6) { if ((mask_str = strchr(ip_strv6, '/'))) { *(mask_str++) = '\0'; - if (strchr(mask_str, '.') != NULL) { + if (strchr(mask_str, ':') != NULL) { return 0; } cidr = atoi(mask_str); - if ((cidr < 0) || (cidr > 64)) { + if ((cidr < 0) || (cidr > 128)) { return 0; } netmask_v6 = (unsigned char)cidr; diff --git a/apache2/re_operators.c b/apache2/re_operators.c index d58b05c2..dc2de445 100644 --- a/apache2/re_operators.c +++ b/apache2/re_operators.c @@ -122,7 +122,7 @@ static int msre_op_ipmatch_param_init(msre_rule *rule, char **error_msg) { * \retval 0 On No Match */ static int msre_op_ipmatch_execute(modsec_rec *msr, msre_rule *rule, msre_var *var, char **error_msg) { - TreeRoot *rtree = rule->ip_op; + TreeRoot *rtree = NULL; int res = 0; if (error_msg == NULL) @@ -130,11 +130,13 @@ static int msre_op_ipmatch_execute(modsec_rec *msr, msre_rule *rule, msre_var *v else *error_msg = NULL; - if (rtree == NULL) { + if (rule == NULL || rule->ip_op == NULL) { msr_log(msr, 1, "ipMatch Internal Error: ipmatch value is null."); return 0; } + rtree = rule->ip_op; + res = tree_contains_ip(msr->mp, rtree, var->value, NULL, error_msg); if (res < 0) {