Actions
Bug #5748
closediprep/ipv6: memory leak on same input in different forms
Affected Versions:
Effort:
Difficulty:
Label:
Description
2001:0000:0000:0000:0000:0000:0000:0000/16,2,100 2001:0470:73f7:0000:0000:0000:0000:0007/16,2,100
Leads to:
================================================================= ==601433==ERROR: LeakSanitizer: detected memory leaks Direct leak of 64 byte(s) in 1 object(s) allocated from: #0 0x564d1381fc9e in malloc (/home/victor/sync/devel/suricata-6.0.x/src/suricata+0x7fac9e) (BuildId: 7a62c6750ba2caf5a87fe129f2c27374db7f1145) #1 0x564d1498fe01 in SCMallocFunc /home/victor/devel/suricata-6.0.x/src/util-mem.c:30:20 #2 0x564d14667671 in SRepCIDRAddNetblock /home/victor/devel/suricata-6.0.x/src/reputation.c:84:22 #3 0x564d14663041 in SRepSplitLine /home/victor/devel/suricata-6.0.x/src/reputation.c:322:9 #4 0x564d14661e4a in SRepLoadFileFromFD /home/victor/devel/suricata-6.0.x/src/reputation.c:467:17 #5 0x564d146642ec in SRepLoadFile /home/victor/devel/suricata-6.0.x/src/reputation.c:430:9 #6 0x564d14663aaf in SRepInit /home/victor/devel/suricata-6.0.x/src/reputation.c:645:25 #7 0x564d13dd2f96 in DetectEngineCtxInitReal /home/victor/devel/suricata-6.0.x/src/detect-engine.c:2057:11 #8 0x564d13dd320d in DetectEngineCtxInit /home/victor/devel/suricata-6.0.x/src/detect-engine.c:2093:12 #9 0x564d14880a4d in PostConfLoadedDetectSetup /home/victor/devel/suricata-6.0.x/src/suricata.c:2441:22 #10 0x564d14884560 in SuricataMain /home/victor/devel/suricata-6.0.x/src/suricata.c:2900:5 #11 0x564d1385aa1e in main /home/victor/devel/suricata-6.0.x/src/main.c:22:12 #12 0x7faf8e382d8f in __libc_start_call_main csu/../sysdeps/nptl/libc_start_call_main.h:58:16 SUMMARY: AddressSanitizer: 64 byte(s) leaked in 1 allocation(s).
Updated by Giuseppe Longo over 1 year ago
Memory leak happens also with IPV4 addresses:
192.168.1.0/16,2,100 192.168.2.0/16,2,100
I'm not sure if an IP address should be added again if it is already present.
In that case, checking if the ip it's already in the tree before adding it prevent the memory to be leaked.
I've quickly made these changes below, and the leaks are fixed.
diff --git a/src/reputation.c b/src/reputation.c
index f22f021c6..3a4d6a1cb 100644
--- a/src/reputation.c
+++ b/src/reputation.c
@@ -101,6 +101,7 @@ static void SRepCIDRAddNetblock(SRepCIDRTree *cidr_ctx, char *ip, int cat, uint8
SCLogDebug("adding ipv6 host %s", ip);
if (SCRadixAddKeyIPV6String(ip, cidr_ctx->srepIPV6_tree[cat], (void *)user_data) == NULL) {
SCLogWarning("failed to add ipv6 host %s", ip);
+ SCFree(user_data);
}
} else {
@@ -116,6 +117,7 @@ static void SRepCIDRAddNetblock(SRepCIDRTree *cidr_ctx, char *ip, int cat, uint8
SCLogDebug("adding ipv4 host %s", ip);
if (SCRadixAddKeyIPV4String(ip, cidr_ctx->srepIPV4_tree[cat], (void *)user_data) == NULL) {
SCLogWarning("failed to add ipv4 host %s", ip);
+ SCFree(user_data);
}
}
}
diff --git a/src/util-radix-tree.c b/src/util-radix-tree.c
index 6192bd480..be7869596 100644
--- a/src/util-radix-tree.c
+++ b/src/util-radix-tree.c
@@ -1007,6 +1007,11 @@ SCRadixNode *SCRadixAddKeyIPV4String(const char *str, SCRadixTree *tree, void *u
SCRadixValidateIPv4Key((uint8_t *)&ip, netmask);
#endif
}
+ if (SCRadixFindKeyIPV4Netblock((uint8_t *)&ip, tree, netmask, user) != NULL) {
+ SCLogWarning("IP already added, skipping it.");
+ return NULL;
+ }
+
return SCRadixAddKey((uint8_t *)&ip, 32, tree, user, netmask);
}
@@ -1072,6 +1077,10 @@ SCRadixNode *SCRadixAddKeyIPV6String(const char *str, SCRadixTree *tree, void *u
SCRadixValidateIPv6Key((uint8_t *)&addr.s6_addr, netmask);
#endif
}
+ if (SCRadixFindKeyIPV6Netblock(addr.s6_addr, tree, netmask, user) != NULL) {
+ SCLogWarning("IP already added, skipping it.");
+ return NULL;
+ }
return SCRadixAddKey(addr.s6_addr, 128, tree, user, netmask);
}
Updated by Jeff Lucovsky over 1 year ago
- Status changed from New to In Review
- Assignee changed from OISF Dev to Jeff Lucovsky
Updated by Jeff Lucovsky over 1 year ago
- Status changed from In Review to Closed
Actions