From b7d459dd2b71b4a4a3ff69296d30cc8766744526 Mon Sep 17 00:00:00 2001 From: Alfredo Cardigliano Date: Fri, 13 Sep 2019 12:36:19 +0200 Subject: [PATCH] Moved strdup for JSON/TLV strings from Flow to Parser to handle non-null-terminated strings --- include/Flow.h | 8 ++++---- include/ParsedFlow.h | 1 - src/NetworkInterface.cpp | 20 +++++++++++++++----- src/ParsedFlow.cpp | 21 ++++++++------------- src/ZMQParserInterface.cpp | 21 ++++++++++++++------- 5 files changed, 41 insertions(+), 30 deletions(-) diff --git a/include/Flow.h b/include/Flow.h index 370061ee74..5e1f452c18 100644 --- a/include/Flow.h +++ b/include/Flow.h @@ -291,8 +291,8 @@ class Flow : public GenericHashEntry { return (isSSL() && protos.ssl.certificate) ? protos.ssl.certificate : host_server_name; } inline char* getBitTorrentHash() { return(bt_hash); }; - inline void setBTHash(char *h) { if(!h) return; if(bt_hash) { free(bt_hash); bt_hash = NULL; }; bt_hash = strdup(h); } - inline void setServerName(char *v) { if(host_server_name) free(host_server_name); host_server_name = strdup(v); } + inline void setBTHash(char *h) { if(!h) return; if(bt_hash) free(bt_hash); bt_hash = h; } + inline void setServerName(char *v) { if(host_server_name) free(host_server_name); host_server_name = v; } void setTcpFlags(u_int8_t flags, bool src2dst_direction); void updateTcpFlags(const struct bpf_timeval *when, u_int8_t flags, bool src2dst_direction); @@ -449,11 +449,11 @@ class Flow : public GenericHashEntry { *_icmp_type = protos.icmp.icmp_type, *_icmp_code = protos.icmp.icmp_code, *_icmp_echo_id = protos.icmp.icmp_echo_id; } inline char* getDNSQuery() { return(isDNS() ? protos.dns.last_query : (char*)""); } - inline void setDNSQuery(char *v) { if(isDNS()) { if(protos.dns.last_query) free(protos.dns.last_query); protos.dns.last_query = strdup(v); } } + inline void setDNSQuery(char *v) { if(isDNS()) { if(protos.dns.last_query) free(protos.dns.last_query); protos.dns.last_query = v; } } inline void setDNSQueryType(u_int16_t t) { if(isDNS()) { protos.dns.last_query_type = t; } } inline void setDNSRetCode(u_int16_t c) { if(isDNS()) { protos.dns.last_return_code = c; } } inline char* getHTTPURL() { return(isHTTP() ? protos.http.last_url : (char*)""); } - inline void setHTTPURL(char *v) { if(isHTTP()) { if(protos.http.last_url) free(protos.http.last_url); protos.http.last_url = strdup(v); } } + inline void setHTTPURL(char *v) { if(isHTTP()) { if(protos.http.last_url) free(protos.http.last_url); protos.http.last_url = v; } } inline void setHTTPRetCode(u_int16_t c) { if(isHTTP()) { protos.http.last_return_code = c; } } inline char* getHTTPContentType() { return(isHTTP() ? protos.http.last_content_type : (char*)""); } inline char* getSSLCertificate() { return(isSSL() ? protos.ssl.certificate : (char*)""); } diff --git a/include/ParsedFlow.h b/include/ParsedFlow.h index fc69531215..a83ac4a533 100644 --- a/include/ParsedFlow.h +++ b/include/ParsedFlow.h @@ -26,7 +26,6 @@ class ParsedFlow : public ParsedFlowCore, public ParsedeBPF { private: - bool parsed_flow_free_memory; bool has_parsed_ebpf; json_object *additional_fields_json; ndpi_serializer *additional_fields_tlv; diff --git a/src/NetworkInterface.cpp b/src/NetworkInterface.cpp index 904d95caf5..b6e16d52ec 100644 --- a/src/NetworkInterface.cpp +++ b/src/NetworkInterface.cpp @@ -1367,23 +1367,33 @@ void NetworkInterface::processFlow(ParsedFlow *zflow, bool zmq_flow) { zflow->pkt_sampling_rate*(zflow->in_pkts+zflow->out_pkts), zflow->pkt_sampling_rate*(zflow->in_bytes+zflow->out_bytes)); - if(zflow->dns_query && zflow->dns_query[0] != '\0' && zflow->dns_query[0] != '\n') + if(zflow->dns_query) { flow->setDNSQuery(zflow->dns_query); + zflow->dns_query = NULL; + } flow->setDNSQueryType(zflow->dns_query_type); flow->setDNSRetCode(zflow->dns_ret_code); - if(zflow->http_url && zflow->http_url[0] != '\0' && zflow->http_url[0] != '\n') + if(zflow->http_url) { flow->setHTTPURL(zflow->http_url); + zflow->http_url = NULL; + } - if(zflow->http_site && zflow->http_site[0] != '\0' && zflow->http_site[0] != '\n') + if(zflow->http_site) { flow->setServerName(zflow->http_site); + zflow->http_site = NULL; + } flow->setHTTPRetCode(zflow->http_ret_code); - if(zflow->ssl_server_name && zflow->ssl_server_name[0] != '\0' && zflow->ssl_server_name[0] != '\n') + if(zflow->ssl_server_name) { flow->setServerName(zflow->ssl_server_name); + zflow->ssl_server_name = NULL; + } - if(zflow->bittorrent_hash && zflow->bittorrent_hash[0] != '\0'&& zflow->bittorrent_hash[0] != '\n') + if(zflow->bittorrent_hash) { flow->setBTHash(zflow->bittorrent_hash); + zflow->bittorrent_hash = NULL; + } if(zflow->vrfId) flow->setVRFid(zflow->vrfId); diff --git a/src/ParsedFlow.cpp b/src/ParsedFlow.cpp index 27ea9c6b88..e1d81a9919 100644 --- a/src/ParsedFlow.cpp +++ b/src/ParsedFlow.cpp @@ -39,7 +39,6 @@ ParsedFlow::ParsedFlow() : ParsedFlowCore(), ParsedeBPF() { memset(&custom_app, 0, sizeof(custom_app)); has_parsed_ebpf = false; - parsed_flow_free_memory = false; } /* *************************************** */ @@ -69,8 +68,6 @@ ParsedFlow::ParsedFlow(const ParsedFlow &pf) : ParsedFlowCore(pf), ParsedeBPF(pf memcpy(&custom_app, &pf.custom_app, sizeof(custom_app)); has_parsed_ebpf = pf.has_parsed_ebpf; - - parsed_flow_free_memory = true; } /* *************************************** */ @@ -84,16 +81,14 @@ ParsedFlow::~ParsedFlow() { free(additional_fields_tlv); } - if(parsed_flow_free_memory) { - if(http_url) free(http_url); - if(http_site) free(http_site); - if(dns_query) free(dns_query); - if(ssl_server_name) free(ssl_server_name); - if(bittorrent_hash) free(bittorrent_hash); - if(ja3c_hash) free(ja3c_hash); - if(ja3s_hash) free(ja3s_hash); - if(suricata_alert) free(suricata_alert); - } + if(http_url) free(http_url); + if(http_site) free(http_site); + if(dns_query) free(dns_query); + if(ssl_server_name) free(ssl_server_name); + if(bittorrent_hash) free(bittorrent_hash); + if(ja3c_hash) free(ja3c_hash); + if(ja3s_hash) free(ja3s_hash); + if(suricata_alert) free(suricata_alert); } /* *************************************** */ diff --git a/src/ZMQParserInterface.cpp b/src/ZMQParserInterface.cpp index bcbeeebcd4..3d042da6a6 100755 --- a/src/ZMQParserInterface.cpp +++ b/src/ZMQParserInterface.cpp @@ -552,7 +552,8 @@ bool ZMQParserInterface::parsePENNtopField(ParsedFlow * const flow, u_int32_t fi flow->tcp.applLatencyMsec = value->double_num; break; case DNS_QUERY: - flow->dns_query = (char *) value->string; + if (value->string[0] && value->string[0] != '\n') + flow->dns_query = strdup(value->string); break; case DNS_QUERY_TYPE: flow->dns_query_type = value->uint_num; @@ -561,22 +562,27 @@ bool ZMQParserInterface::parsePENNtopField(ParsedFlow * const flow, u_int32_t fi flow->dns_ret_code = value->uint_num; break; case HTTP_URL: - flow->http_url = (char *) value->string; + if (value->string[0] && value->string[0] != '\n') + flow->http_url = strdup(value->string); break; case HTTP_SITE: - flow->http_site = (char *) value->string; + if (value->string[0] && value->string[0] != '\n') + flow->http_site = strdup(value->string); break; case HTTP_RET_CODE: flow->http_ret_code = value->uint_num; break; case SSL_SERVER_NAME: - flow->ssl_server_name = (char *) value->string; + if (value->string[0] && value->string[0] != '\n') + flow->ssl_server_name = strdup(value->string); break; case JA3C_HASH: - flow->ja3c_hash = (char *) value->string; + if (value->string[0]) + flow->ja3c_hash = strdup(value->string); break; case JA3S_HASH: - flow->ja3s_hash = (char *) value->string; + if (value->string[0]) + flow->ja3s_hash = strdup(value->string); break; case SSL__CIPHER: flow->ssl_cipher = value->uint_num; @@ -585,7 +591,8 @@ bool ZMQParserInterface::parsePENNtopField(ParsedFlow * const flow, u_int32_t fi flow->ssl_unsafe_cipher = value->uint_num; break; case BITTORRENT_HASH: - flow->bittorrent_hash = (char *) value->string; + if (value->string[0] && value->string[0] != '\n') + flow->bittorrent_hash = strdup(value->string); break; case NPROBE_IPV4_ADDRESS: /* Do not override EXPORTER_IPV4_ADDRESS */