From dd377fffba04ab3fdd4601befd59a10090c48aeb Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Sun, 16 Feb 2020 19:11:08 +0100 Subject: [PATCH 1/3] analyzer/protocol/dns: fix memory leak If `dns_TSIG_addl` is not set, then the BroString allocated by ExtractOctets() leaks. Therefore, don't ask ExtractOctets() to copy the data to a BroString if we're not going to use it. Yet another memory leak (out of way too many) which would have been prevented by using smart pointers. --- src/analyzer/protocol/dns/DNS.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/analyzer/protocol/dns/DNS.cc b/src/analyzer/protocol/dns/DNS.cc index db072ffeb3..21b4a0161f 100644 --- a/src/analyzer/protocol/dns/DNS.cc +++ b/src/analyzer/protocol/dns/DNS.cc @@ -769,7 +769,7 @@ int DNS_Interpreter::ParseRR_TSIG(DNS_MsgInfo* msg, unsigned int sign_time_msec = ExtractShort(data, len); unsigned int fudge = ExtractShort(data, len); BroString* request_MAC; - ExtractOctets(data, len, &request_MAC); + ExtractOctets(data, len, dns_TSIG_addl ? &request_MAC : nullptr); unsigned int orig_id = ExtractShort(data, len); unsigned int rr_error = ExtractShort(data, len); ExtractOctets(data, len, 0); // Other Data From aac3de576fd6b95662f28ff5f02fd797bc932f41 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Sun, 16 Feb 2020 19:11:10 +0100 Subject: [PATCH 2/3] analyzer/protocol/dns: change runtime check to assert() If it were legal to call SendReplyOrRejectEvent() without an EventHandlerPtr, then this would leak the `question_name` object. But this method has just one caller, and it verifies the EventHandlerPtr. --- src/analyzer/protocol/dns/DNS.cc | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/src/analyzer/protocol/dns/DNS.cc b/src/analyzer/protocol/dns/DNS.cc index 21b4a0161f..38bbd9ce00 100644 --- a/src/analyzer/protocol/dns/DNS.cc +++ b/src/analyzer/protocol/dns/DNS.cc @@ -1415,14 +1415,15 @@ void DNS_Interpreter::SendReplyOrRejectEvent(DNS_MsgInfo* msg, RR_Type qtype = RR_Type(ExtractShort(data, len)); int qclass = ExtractShort(data, len); - if ( event ) - analyzer->ConnectionEventFast(event, { - analyzer->BuildConnVal(), - msg->BuildHdrVal(), - new StringVal(question_name), - val_mgr->GetCount(qtype), - val_mgr->GetCount(qclass), - }); + assert(event); + + analyzer->ConnectionEventFast(event, { + analyzer->BuildConnVal(), + msg->BuildHdrVal(), + new StringVal(question_name), + val_mgr->GetCount(qtype), + val_mgr->GetCount(qclass), + }); } From 957e7d3245295095190ad7895f858562b63551eb Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Mon, 17 Feb 2020 19:27:07 +0100 Subject: [PATCH 3/3] analyzer/protocol/dns: fix NSEC3 memory leak --- src/analyzer/protocol/dns/DNS.cc | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/analyzer/protocol/dns/DNS.cc b/src/analyzer/protocol/dns/DNS.cc index 38bbd9ce00..38293f92ac 100644 --- a/src/analyzer/protocol/dns/DNS.cc +++ b/src/analyzer/protocol/dns/DNS.cc @@ -1129,6 +1129,8 @@ int DNS_Interpreter::ParseRR_NSEC3(DNS_MsgInfo* msg, msg->BuildNSEC3_Val(&nsec3), }); } + else + Unref(char_strings); return 1; }