Skip to content

Commit 5d92e44

Browse files
author
Felipe Zimmerle
committed
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.
1 parent 731466c commit 5d92e44

File tree

3 files changed

+36
-13
lines changed

3 files changed

+36
-13
lines changed

apache2/msc_tree.c

Lines changed: 30 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -820,35 +820,46 @@ TreeNode *TreeAddIP(const char *buffer, CPTTree *tree, int type) {
820820
char ip_strv4[NETMASK_32], ip_strv6[NETMASK_128];
821821
struct in_addr addr4;
822822
struct in6_addr addr6;
823+
int pos = 0;
823824
char *ptr = NULL;
824825

825826
if(tree == NULL)
826827
return NULL;
827828

829+
pos = strchr(buffer, '/') - buffer;
830+
828831
switch(type) {
829832

830833
case IPV4_TREE:
831834
memset(&addr4, 0, sizeof(addr4));
832835
memset(ip_strv4, 0x0, NETMASK_32);
833836

834-
strncpy(ip_strv4, buffer, sizeof(ip_strv4) - 2);
837+
strncpy(ip_strv4, buffer, sizeof(ip_strv4));
835838
*(ip_strv4 + (sizeof(ip_strv4) - 1)) = '\0';
836839

837840
ptr = strdup(ip_strv4);
838841
netmask_v4 = is_netmask_v4(ptr);
839842

843+
if (netmask_v4 > NETMASK_32) {
844+
free(ptr);
845+
ptr = NULL;
846+
return NULL;
847+
}
848+
840849
if(ptr != NULL) {
841850
free(ptr);
842851
ptr = NULL;
843852
}
844853

845-
if(netmask_v4 == 0)
854+
if(netmask_v4 == 0) {
846855
return NULL;
847-
else if(netmask_v4 != NETMASK_32) {
848-
ip_strv4[strlen(ip_strv4)-3] = '\0';
856+
}
857+
else if (netmask_v4 != NETMASK_32 && pos < strlen(ip_strv4)) {
858+
ip_strv4[pos] = '\0';
849859
}
850860

851861
ret = inet_pton(AF_INET, ip_strv4, &addr4);
862+
852863
if (ret <= 0) {
853864
return NULL;
854865
}
@@ -863,26 +874,36 @@ TreeNode *TreeAddIP(const char *buffer, CPTTree *tree, int type) {
863874
memset(&addr6, 0, sizeof(addr6));
864875
memset(ip_strv6, 0x0, NETMASK_128);
865876

866-
strncpy(ip_strv6, buffer, sizeof(ip_strv6) - 2);
877+
strncpy(ip_strv6, buffer, sizeof(ip_strv6));
867878
*(ip_strv6 + sizeof(ip_strv6) - 1) = '\0';
868879

869880
ptr = strdup(ip_strv6);
870881
netmask_v6 = is_netmask_v6(ptr);
871882

883+
if (netmask_v6 > NETMASK_128) {
884+
free(ptr);
885+
ptr = NULL;
886+
return NULL;
887+
}
888+
872889
if(ptr != NULL) {
873890
free(ptr);
874891
ptr = NULL;
875892
}
876893

877-
if(netmask_v6 == 0)
894+
if(netmask_v6 == 0) {
878895
return NULL;
879-
else if (netmask_v6 != NETMASK_64) {
880-
ip_strv4[strlen(ip_strv6)-3] = '\0';
896+
}
897+
else if (netmask_v6 != NETMASK_128 && pos < strlen(ip_strv6)) {
898+
ip_strv6[pos] = '\0';
881899
}
882900

883901
ret = inet_pton(AF_INET6, ip_strv6, &addr6);
884-
if(ret <= 0)
902+
903+
if (ret <= 0)
904+
{
885905
return NULL;
906+
}
886907

887908
tree->count++;
888909

apache2/msc_util.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -364,12 +364,12 @@ unsigned char is_netmask_v6(char *ip_strv6) {
364364
if ((mask_str = strchr(ip_strv6, '/'))) {
365365
*(mask_str++) = '\0';
366366

367-
if (strchr(mask_str, '.') != NULL) {
367+
if (strchr(mask_str, ':') != NULL) {
368368
return 0;
369369
}
370370

371371
cidr = atoi(mask_str);
372-
if ((cidr < 0) || (cidr > 64)) {
372+
if ((cidr < 0) || (cidr > 128)) {
373373
return 0;
374374
}
375375
netmask_v6 = (unsigned char)cidr;

apache2/re_operators.c

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -122,19 +122,21 @@ static int msre_op_ipmatch_param_init(msre_rule *rule, char **error_msg) {
122122
* \retval 0 On No Match
123123
*/
124124
static int msre_op_ipmatch_execute(modsec_rec *msr, msre_rule *rule, msre_var *var, char **error_msg) {
125-
TreeRoot *rtree = rule->ip_op;
125+
TreeRoot *rtree = NULL;
126126
int res = 0;
127127

128128
if (error_msg == NULL)
129129
return -1;
130130
else
131131
*error_msg = NULL;
132132

133-
if (rtree == NULL) {
133+
if (rule == NULL || rule->ip_op == NULL) {
134134
msr_log(msr, 1, "ipMatch Internal Error: ipmatch value is null.");
135135
return 0;
136136
}
137137

138+
rtree = rule->ip_op;
139+
138140
res = tree_contains_ip(msr->mp, rtree, var->value, NULL, error_msg);
139141

140142
if (res < 0) {

0 commit comments

Comments
 (0)