From c2ee15b09f0ecf3701be68844d58b781bd517216 Mon Sep 17 00:00:00 2001 From: Julien Sentier Date: Thu, 23 Feb 2012 12:48:28 +0100 Subject: [PATCH 1/8] protection from bad frees on unallocated strings --- src/BroDoc.cc | 4 ++-- src/Debug.cc | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/BroDoc.cc b/src/BroDoc.cc index b20db727ff..9c617f78b3 100644 --- a/src/BroDoc.cc +++ b/src/BroDoc.cc @@ -85,8 +85,8 @@ void BroDoc::AddImport(const std::string& s) if ( ext_pos != std::string::npos ) lname = lname.substr(0, ext_pos); - const char* full_filename = ""; - const char* subpath = ""; + const char* full_filename = NULL; + const char* subpath = NULL; FILE* f = search_for_file(lname.c_str(), "bro", &full_filename, true, &subpath); diff --git a/src/Debug.cc b/src/Debug.cc index 1b849fff7e..ea9c52f77e 100644 --- a/src/Debug.cc +++ b/src/Debug.cc @@ -142,7 +142,7 @@ int TraceState::LogTrace(const char* fmt, ...) if ( ! loc.filename ) { - loc.filename = ""; + loc.filename = copy_string(""); loc.last_line = 0; } @@ -735,7 +735,7 @@ string get_context_description(const Stmt* stmt, const Frame* frame) loc = *stmt->GetLocationInfo(); else { - loc.filename = ""; + loc.filename = copy_string(""); loc.last_line = 0; } From b84fd05912d27f9466e661df777a9a5ed5ade6e0 Mon Sep 17 00:00:00 2001 From: Julien Sentier Date: Thu, 23 Feb 2012 13:01:22 +0100 Subject: [PATCH 2/8] Better use of operators priorities --- src/ConnCompressor.cc | 2 +- src/LogMgr.cc | 2 +- src/StateAccess.cc | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/ConnCompressor.cc b/src/ConnCompressor.cc index 9fa43bd3a8..29e24457f5 100644 --- a/src/ConnCompressor.cc +++ b/src/ConnCompressor.cc @@ -440,7 +440,7 @@ Connection* ConnCompressor::NextFromOrig(PendingConn* pending, double t, else if ( tp->th_flags & TH_SYN ) { - if ( ! tp->th_flags & TH_ACK ) + if ( ! (tp->th_flags & TH_ACK) ) { Weird(pending, t, "SYN_after_partial"); pending->SYN = 1; diff --git a/src/LogMgr.cc b/src/LogMgr.cc index 9d397325be..bebce97352 100644 --- a/src/LogMgr.cc +++ b/src/LogMgr.cc @@ -911,7 +911,7 @@ bool LogMgr::Write(EnumVal* id, RecordVal* columns) return false; } - if ( ! v->Type()->Tag() == TYPE_STRING ) + if ( v->Type()->Tag() != TYPE_STRING ) { reporter->Error("path_func did not return string"); Unref(v); diff --git a/src/StateAccess.cc b/src/StateAccess.cc index 136a006c52..7abef72c46 100644 --- a/src/StateAccess.cc +++ b/src/StateAccess.cc @@ -231,7 +231,7 @@ bool StateAccess::CheckOldSet(const char* op, ID* id, Val* index, bool StateAccess::MergeTables(TableVal* dst, Val* src) { - if ( ! src->Type()->Tag() == TYPE_TABLE ) + if ( src->Type()->Tag() != TYPE_TABLE ) { reporter->Error("type mismatch while merging tables"); return false; From 7dfb5657a29c589d9ead3dd5179b7efd3a405676 Mon Sep 17 00:00:00 2001 From: Julien Sentier Date: Thu, 23 Feb 2012 12:58:52 +0100 Subject: [PATCH 3/8] Good overridance with the good qualifier --- src/Trigger.cc | 2 +- src/Trigger.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Trigger.cc b/src/Trigger.cc index 26f80c73d9..164f11b885 100644 --- a/src/Trigger.cc +++ b/src/Trigger.cc @@ -410,7 +410,7 @@ Val* Trigger::Lookup(const CallExpr* expr) return (i != cache.end()) ? i->second : 0; } -const char* Trigger::Name() +const char* Trigger::Name() const { assert(location); return fmt("%s:%d-%d", location->filename, diff --git a/src/Trigger.h b/src/Trigger.h index ffec50d7ef..8e04fb9189 100644 --- a/src/Trigger.h +++ b/src/Trigger.h @@ -60,7 +60,7 @@ public: virtual void Access(Val* val, const StateAccess& sa) { QueueTrigger(this); } - virtual const char* Name(); + virtual const char* Name() const; static void QueueTrigger(Trigger* trigger); From 1df650eb0d4cffd548d21a2131ac60f4cd14a162 Mon Sep 17 00:00:00 2001 From: Julien Sentier Date: Thu, 23 Feb 2012 13:21:47 +0100 Subject: [PATCH 4/8] A destructor must free the memory allocated by the constructor --- src/Gnutella.cc | 6 ++++++ src/Gnutella.h | 1 + src/NCP.cc | 2 ++ src/Sessions.cc | 4 ++++ 4 files changed, 13 insertions(+) diff --git a/src/Gnutella.cc b/src/Gnutella.cc index 448c8dcb3b..6b5e901bc5 100644 --- a/src/Gnutella.cc +++ b/src/Gnutella.cc @@ -42,6 +42,12 @@ Gnutella_Analyzer::Gnutella_Analyzer(Connection* conn) resp_msg_state = new GnutellaMsgState(); } +Gnutella_Analyzer::~Gnutella_Analyzer() + { + delete orig_msg_state; + delete resp_msg_state; + } + void Gnutella_Analyzer::Done() { TCP_ApplicationAnalyzer::Done(); diff --git a/src/Gnutella.h b/src/Gnutella.h index f06c816c90..455876462d 100644 --- a/src/Gnutella.h +++ b/src/Gnutella.h @@ -35,6 +35,7 @@ public: class Gnutella_Analyzer : public TCP_ApplicationAnalyzer { public: Gnutella_Analyzer(Connection* conn); + ~Gnutella_Analyzer(); virtual void Done (); virtual void DeliverStream(int len, const u_char* data, bool orig); diff --git a/src/NCP.cc b/src/NCP.cc index 83378a09a7..edd882747c 100644 --- a/src/NCP.cc +++ b/src/NCP.cc @@ -225,5 +225,7 @@ NCP_Analyzer::NCP_Analyzer(Connection* conn) NCP_Analyzer::~NCP_Analyzer() { delete session; + delete o_ncp; + delete r_ncp; } diff --git a/src/Sessions.cc b/src/Sessions.cc index d78032a25b..b78fdd67d0 100644 --- a/src/Sessions.cc +++ b/src/Sessions.cc @@ -135,6 +135,10 @@ NetSessions::~NetSessions() delete SYN_OS_Fingerprinter; delete pkt_profiler; Unref(arp_analyzer); + if (discarder) + delete discarder; + if (stp_manager) + delete stp_manager; } void NetSessions::Done() From a3e419fee052d8ca9fdfcb27cd27b92889605a2a Mon Sep 17 00:00:00 2001 From: Julien Sentier Date: Thu, 23 Feb 2012 13:26:48 +0100 Subject: [PATCH 5/8] removing dead code --- src/Type.cc | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/src/Type.cc b/src/Type.cc index 4d80eda6f7..82221303af 100644 --- a/src/Type.cc +++ b/src/Type.cc @@ -1858,13 +1858,8 @@ BroType* merge_types(const BroType* t1, const BroType* t2) if ( t1->IsSet() ) return new SetType(tl3, 0); - else if ( tg1 == TYPE_TABLE ) - return new TableType(tl3, y3); else - { - reporter->InternalError("bad tag in merge_types"); - return 0; - } + return new TableType(tl3, y3); } case TYPE_FUNC: From 2e069c9596a1da00ffcdacfae47eda55522cdbc3 Mon Sep 17 00:00:00 2001 From: Julien Sentier Date: Thu, 23 Feb 2012 14:07:15 +0100 Subject: [PATCH 6/8] Fix of some memory leaks --- src/Expr.cc | 1 + src/Net.cc | 1 + src/NetbiosSSN.cc | 3 +++ src/POP3.cc | 2 ++ src/RuleMatcher.cc | 5 +++++ 5 files changed, 12 insertions(+) diff --git a/src/Expr.cc b/src/Expr.cc index a8c2fc32c9..1ab49bcd3b 100644 --- a/src/Expr.cc +++ b/src/Expr.cc @@ -4910,6 +4910,7 @@ Val* ListExpr::Eval(Frame* f) const if ( ! ev ) { Error("uninitialized list value"); + delete v; return 0; } diff --git a/src/Net.cc b/src/Net.cc index 2d8ee85353..149cee6e94 100644 --- a/src/Net.cc +++ b/src/Net.cc @@ -254,6 +254,7 @@ void net_init(name_list& interfaces, name_list& readfiles, { io_sources.Register(fs); } + delete fs; } } diff --git a/src/NetbiosSSN.cc b/src/NetbiosSSN.cc index 274e76f137..7f7a77933d 100644 --- a/src/NetbiosSSN.cc +++ b/src/NetbiosSSN.cc @@ -131,6 +131,9 @@ int NetbiosSSN_Interpreter::ParseBroadcast(const u_char* data, int len, return 0; } + delete srcname; + delete dstname; + return 0; } diff --git a/src/POP3.cc b/src/POP3.cc index 4ffe67ef48..3075e76507 100644 --- a/src/POP3.cc +++ b/src/POP3.cc @@ -158,6 +158,7 @@ void POP3_Analyzer::ProcessRequest(int length, const char* line) if ( e >= end ) { Weird("pop3_malformed_auth_plain"); + delete decoded; return; } @@ -167,6 +168,7 @@ void POP3_Analyzer::ProcessRequest(int length, const char* line) if ( s >= end ) { Weird("pop3_malformed_auth_plain"); + delete decoded; return; } diff --git a/src/RuleMatcher.cc b/src/RuleMatcher.cc index 685e35bade..9be559c5ab 100644 --- a/src/RuleMatcher.cc +++ b/src/RuleMatcher.cc @@ -73,6 +73,7 @@ RuleHdrTest::RuleHdrTest(RuleHdrTest& h) copied_set->ids = orig_set->ids; loop_over_list(orig_set->patterns, l) copied_set->patterns.append(copy_string(orig_set->patterns[l])); + delete copied_set; } } @@ -1116,7 +1117,11 @@ void id_to_maskedvallist(const char* id, maskedvalue_list* append_to) val_list* vals = v->AsTableVal()->ConvertToPureList()->Vals(); loop_over_list(*vals, i ) if ( ! val_to_maskedval((*vals)[i], append_to) ) + { + delete vals; return; + } + delete vals; } else From d65b2f12c66fab10d1c7f90bba11bfe1df30d1a4 Mon Sep 17 00:00:00 2001 From: Julien Sentier Date: Thu, 23 Feb 2012 14:15:56 +0100 Subject: [PATCH 7/8] Suppression of unused code --- src/Expr.cc | 2 -- src/SMTP.cc | 1 - src/TCP.cc | 1 - 3 files changed, 4 deletions(-) diff --git a/src/Expr.cc b/src/Expr.cc index 1ab49bcd3b..f8d2772feb 100644 --- a/src/Expr.cc +++ b/src/Expr.cc @@ -2664,8 +2664,6 @@ void AssignExpr::EvalIntoAggregate(const BroType* t, Val* aggr, Frame* f) const Error("bad table insertion"); TableVal* tv = aggr->AsTableVal(); - const TableType* tt = tv->Type()->AsTableType(); - const BroType* yt = tv->Type()->YieldType(); Val* index = op1->Eval(f); Val* v = op2->Eval(f); diff --git a/src/SMTP.cc b/src/SMTP.cc index 3af8af3b7b..85a3bc79dc 100644 --- a/src/SMTP.cc +++ b/src/SMTP.cc @@ -353,7 +353,6 @@ void SMTP_Analyzer::ProcessLine(int length, const char* line, bool orig) int ext_len; get_word(end_of_line - line, line, ext_len, ext); - line = skip_whitespace(line + ext_len, end_of_line); ProcessExtension(ext_len, ext); } } diff --git a/src/TCP.cc b/src/TCP.cc index dc71d13252..3315db79f3 100644 --- a/src/TCP.cc +++ b/src/TCP.cc @@ -990,7 +990,6 @@ void TCP_Analyzer::DeliverPacket(int len, const u_char* data, bool is_orig, Conn()->SetLastTime(t); const IPAddr orig_addr = Conn()->OrigAddr(); - const IPAddr resp_addr = Conn()->RespAddr(); uint32 tcp_hdr_len = data - (const u_char*) tp; From 900cc8f2ab81d7a2808cdfb6281aff951a9730d6 Mon Sep 17 00:00:00 2001 From: Julien Sentier Date: Thu, 23 Feb 2012 14:17:09 +0100 Subject: [PATCH 8/8] possible use after free forbidden --- src/PktSrc.cc | 1 + 1 file changed, 1 insertion(+) diff --git a/src/PktSrc.cc b/src/PktSrc.cc index 68b9785e6f..422ed3b39e 100644 --- a/src/PktSrc.cc +++ b/src/PktSrc.cc @@ -382,6 +382,7 @@ void PktSrc::AddSecondaryTablePrograms() { delete program; Close(); + continue; } SecondaryProgram* sp = new SecondaryProgram(program, se);