Merge branch 'master-merge-helper'

* master-merge-helper:
  possible use after free forbidden
  Suppression of unused code
  Fix of some memory leaks
  removing dead code
  A destructor must free the memory allocated by the constructor
  Good overridance with the good qualifier
  Better use of operators priorities
  protection from bad frees on unallocated strings
This commit is contained in:
Robin Sommer 2012-02-24 16:34:17 -08:00
commit ada5f38d04
22 changed files with 67 additions and 30 deletions

16
CHANGES
View file

@ -1,4 +1,20 @@
2.0-121 | 2012-02-24 16:34:17 -0800
* A number of smaller memory fixes and code cleanups. (Julien
Sentier)
* Add to_subnet bif. Fixes #782). (Jon Siwek)
* Fix IPAddr::Mask/ReverseMask not allowing argument of 0. (Jon
Siwek)
* Refactor IPAddr v4 initialization from string. Fixes #775. (Jon Siwek)
* Parse the dotted address string directly instead of canonicalizing
and passing to inet_pton. (Jon Siwek)
2.0-108 | 2012-02-24 15:21:07 -0800 2.0-108 | 2012-02-24 15:21:07 -0800
* Refactoring a number of usages of new IPAddr class. (Jon Siwek) * Refactoring a number of usages of new IPAddr class. (Jon Siwek)

View file

@ -1 +1 @@
2.0-108 2.0-121

View file

@ -85,12 +85,13 @@ void BroDoc::AddImport(const std::string& s)
if ( ext_pos != std::string::npos ) if ( ext_pos != std::string::npos )
lname = lname.substr(0, ext_pos); lname = lname.substr(0, ext_pos);
const char* full_filename = "<error>"; const char* full_filename = NULL;
const char* subpath = "<error>"; const char* subpath = NULL;
FILE* f = search_for_file(lname.c_str(), "bro", &full_filename, true, FILE* f = search_for_file(lname.c_str(), "bro", &full_filename, true,
&subpath); &subpath);
if ( f ) if ( f && full_filename && subpath )
{ {
fclose(f); fclose(f);
@ -126,12 +127,14 @@ void BroDoc::AddImport(const std::string& s)
} }
delete [] tmp; delete [] tmp;
delete [] full_filename;
delete [] subpath;
} }
else else
fprintf(stderr, "Failed to document '@load %s' in file: %s\n", fprintf(stderr, "Failed to document '@load %s' in file: %s\n",
s.c_str(), reST_filename.c_str()); s.c_str(), reST_filename.c_str());
delete [] full_filename;
delete [] subpath;
} }
void BroDoc::SetPacketFilter(const std::string& s) void BroDoc::SetPacketFilter(const std::string& s)

View file

@ -440,7 +440,7 @@ Connection* ConnCompressor::NextFromOrig(PendingConn* pending, double t,
else if ( tp->th_flags & TH_SYN ) 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"); Weird(pending, t, "SYN_after_partial");
pending->SYN = 1; pending->SYN = 1;

View file

@ -142,7 +142,7 @@ int TraceState::LogTrace(const char* fmt, ...)
if ( ! loc.filename ) if ( ! loc.filename )
{ {
loc.filename = "<no filename>"; loc.filename = copy_string("<no filename>");
loc.last_line = 0; loc.last_line = 0;
} }
@ -735,7 +735,7 @@ string get_context_description(const Stmt* stmt, const Frame* frame)
loc = *stmt->GetLocationInfo(); loc = *stmt->GetLocationInfo();
else else
{ {
loc.filename = "<no filename>"; loc.filename = copy_string("<no filename>");
loc.last_line = 0; loc.last_line = 0;
} }

View file

@ -2664,8 +2664,6 @@ void AssignExpr::EvalIntoAggregate(const BroType* t, Val* aggr, Frame* f) const
Error("bad table insertion"); Error("bad table insertion");
TableVal* tv = aggr->AsTableVal(); TableVal* tv = aggr->AsTableVal();
const TableType* tt = tv->Type()->AsTableType();
const BroType* yt = tv->Type()->YieldType();
Val* index = op1->Eval(f); Val* index = op1->Eval(f);
Val* v = op2->Eval(f); Val* v = op2->Eval(f);
@ -4910,6 +4908,7 @@ Val* ListExpr::Eval(Frame* f) const
if ( ! ev ) if ( ! ev )
{ {
Error("uninitialized list value"); Error("uninitialized list value");
Unref(v);
return 0; return 0;
} }

View file

@ -42,6 +42,12 @@ Gnutella_Analyzer::Gnutella_Analyzer(Connection* conn)
resp_msg_state = new GnutellaMsgState(); resp_msg_state = new GnutellaMsgState();
} }
Gnutella_Analyzer::~Gnutella_Analyzer()
{
delete orig_msg_state;
delete resp_msg_state;
}
void Gnutella_Analyzer::Done() void Gnutella_Analyzer::Done()
{ {
TCP_ApplicationAnalyzer::Done(); TCP_ApplicationAnalyzer::Done();

View file

@ -35,6 +35,7 @@ public:
class Gnutella_Analyzer : public TCP_ApplicationAnalyzer { class Gnutella_Analyzer : public TCP_ApplicationAnalyzer {
public: public:
Gnutella_Analyzer(Connection* conn); Gnutella_Analyzer(Connection* conn);
~Gnutella_Analyzer();
virtual void Done (); virtual void Done ();
virtual void DeliverStream(int len, const u_char* data, bool orig); virtual void DeliverStream(int len, const u_char* data, bool orig);

View file

@ -911,7 +911,7 @@ bool LogMgr::Write(EnumVal* id, RecordVal* columns)
return false; return false;
} }
if ( ! v->Type()->Tag() == TYPE_STRING ) if ( v->Type()->Tag() != TYPE_STRING )
{ {
reporter->Error("path_func did not return string"); reporter->Error("path_func did not return string");
Unref(v); Unref(v);

View file

@ -225,5 +225,7 @@ NCP_Analyzer::NCP_Analyzer(Connection* conn)
NCP_Analyzer::~NCP_Analyzer() NCP_Analyzer::~NCP_Analyzer()
{ {
delete session; delete session;
delete o_ncp;
delete r_ncp;
} }

View file

@ -248,12 +248,14 @@ void net_init(name_list& interfaces, name_list& readfiles,
FlowSocketSrc* fs = new FlowSocketSrc(netflows[i]); FlowSocketSrc* fs = new FlowSocketSrc(netflows[i]);
if ( ! fs->IsOpen() ) if ( ! fs->IsOpen() )
{
reporter->Error("%s: problem with netflow socket %s - %s\n", reporter->Error("%s: problem with netflow socket %s - %s\n",
prog, netflows[i], fs->ErrorMsg()); prog, netflows[i], fs->ErrorMsg());
else delete fs;
{
io_sources.Register(fs);
} }
else
io_sources.Register(fs);
} }
} }

View file

@ -131,6 +131,9 @@ int NetbiosSSN_Interpreter::ParseBroadcast(const u_char* data, int len,
return 0; return 0;
} }
delete srcname;
delete dstname;
return 0; return 0;
} }

View file

@ -158,6 +158,7 @@ void POP3_Analyzer::ProcessRequest(int length, const char* line)
if ( e >= end ) if ( e >= end )
{ {
Weird("pop3_malformed_auth_plain"); Weird("pop3_malformed_auth_plain");
delete decoded;
return; return;
} }
@ -167,6 +168,7 @@ void POP3_Analyzer::ProcessRequest(int length, const char* line)
if ( s >= end ) if ( s >= end )
{ {
Weird("pop3_malformed_auth_plain"); Weird("pop3_malformed_auth_plain");
delete decoded;
return; return;
} }

View file

@ -382,6 +382,7 @@ void PktSrc::AddSecondaryTablePrograms()
{ {
delete program; delete program;
Close(); Close();
return;
} }
SecondaryProgram* sp = new SecondaryProgram(program, se); SecondaryProgram* sp = new SecondaryProgram(program, se);

View file

@ -73,6 +73,9 @@ RuleHdrTest::RuleHdrTest(RuleHdrTest& h)
copied_set->ids = orig_set->ids; copied_set->ids = orig_set->ids;
loop_over_list(orig_set->patterns, l) loop_over_list(orig_set->patterns, l)
copied_set->patterns.append(copy_string(orig_set->patterns[l])); copied_set->patterns.append(copy_string(orig_set->patterns[l]));
delete copied_set;
// TODO: Why do we create copied_set only to then
// never use it?
} }
} }
@ -1116,7 +1119,12 @@ void id_to_maskedvallist(const char* id, maskedvalue_list* append_to)
val_list* vals = v->AsTableVal()->ConvertToPureList()->Vals(); val_list* vals = v->AsTableVal()->ConvertToPureList()->Vals();
loop_over_list(*vals, i ) loop_over_list(*vals, i )
if ( ! val_to_maskedval((*vals)[i], append_to) ) if ( ! val_to_maskedval((*vals)[i], append_to) )
{
delete_vals(vals);
return; return;
}
delete_vals(vals);
} }
else else

View file

@ -352,8 +352,8 @@ void SMTP_Analyzer::ProcessLine(int length, const char* line, bool orig)
const char* ext; const char* ext;
int ext_len; int ext_len;
get_word(end_of_line - line, line, ext_len, ext);
line = skip_whitespace(line + ext_len, end_of_line); line = skip_whitespace(line + ext_len, end_of_line);
get_word(end_of_line - line, line, ext_len, ext);
ProcessExtension(ext_len, ext); ProcessExtension(ext_len, ext);
} }
} }

View file

@ -135,12 +135,12 @@ NetSessions::~NetSessions()
delete SYN_OS_Fingerprinter; delete SYN_OS_Fingerprinter;
delete pkt_profiler; delete pkt_profiler;
Unref(arp_analyzer); Unref(arp_analyzer);
delete discarder;
delete stp_manager;
} }
void NetSessions::Done() void NetSessions::Done()
{ {
delete stp_manager;
delete discarder;
} }
namespace // private namespace namespace // private namespace

View file

@ -231,7 +231,7 @@ bool StateAccess::CheckOldSet(const char* op, ID* id, Val* index,
bool StateAccess::MergeTables(TableVal* dst, Val* src) 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"); reporter->Error("type mismatch while merging tables");
return false; return false;

View file

@ -990,7 +990,6 @@ void TCP_Analyzer::DeliverPacket(int len, const u_char* data, bool is_orig,
Conn()->SetLastTime(t); Conn()->SetLastTime(t);
const IPAddr orig_addr = Conn()->OrigAddr(); const IPAddr orig_addr = Conn()->OrigAddr();
const IPAddr resp_addr = Conn()->RespAddr();
uint32 tcp_hdr_len = data - (const u_char*) tp; uint32 tcp_hdr_len = data - (const u_char*) tp;

View file

@ -410,7 +410,7 @@ Val* Trigger::Lookup(const CallExpr* expr)
return (i != cache.end()) ? i->second : 0; return (i != cache.end()) ? i->second : 0;
} }
const char* Trigger::Name() const char* Trigger::Name() const
{ {
assert(location); assert(location);
return fmt("%s:%d-%d", location->filename, return fmt("%s:%d-%d", location->filename,

View file

@ -60,7 +60,7 @@ public:
virtual void Access(Val* val, const StateAccess& sa) virtual void Access(Val* val, const StateAccess& sa)
{ QueueTrigger(this); } { QueueTrigger(this); }
virtual const char* Name(); virtual const char* Name() const;
static void QueueTrigger(Trigger* trigger); static void QueueTrigger(Trigger* trigger);

View file

@ -1858,13 +1858,8 @@ BroType* merge_types(const BroType* t1, const BroType* t2)
if ( t1->IsSet() ) if ( t1->IsSet() )
return new SetType(tl3, 0); return new SetType(tl3, 0);
else if ( tg1 == TYPE_TABLE )
return new TableType(tl3, y3);
else else
{ return new TableType(tl3, y3);
reporter->InternalError("bad tag in merge_types");
return 0;
}
} }
case TYPE_FUNC: case TYPE_FUNC: