Fix various potential memory leaks.

Though I expect most not to be exercised in practice.

src/BroDoc.cc

@@ -99,8 +99,6 @@ void BroDoc::AddImport(const std::string& s)
 
 	if ( f && full_filename && subpath )
 		{
-		fclose(f);
-
 		char* tmp = copy_string(full_filename);
 		char* filename = basename(tmp);
 		extern char* PACKAGE_LOADER;
@@ -139,6 +137,9 @@ void BroDoc::AddImport(const std::string& s)
 		fprintf(stderr, "Failed to document '@load %s' in file: %s\n",
 		        s.c_str(), reST_filename.c_str());
 
+	if ( f )
+		fclose(f);
+
 	delete [] full_filename;
 	delete [] subpath;
 	}
@@ -559,7 +560,7 @@ static void WriteAnalyzerTagDefn(FILE* f, EnumType* e, const string& module)
 	e = new CommentedEnumType(e);
 	e->SetTypeID(copy_string(tag_id.c_str()));
 
-	ID* dummy_id = new ID(copy_string(tag_id.c_str()), SCOPE_GLOBAL, true);
+	ID* dummy_id = new ID(tag_id.c_str(), SCOPE_GLOBAL, true);
 	dummy_id->SetType(e);
 	dummy_id->MakeType();
 

src/Conn.cc

@@ -527,7 +527,13 @@ Val* Connection::BuildVersionVal(const char* s, int len)
 
 	// We need at least a name.
 	if ( ! name )
+		{
+		Unref(major);
+		Unref(minor);
+		Unref(minor2);
+		Unref(addl);
 		return 0;
+		}
 
 	RecordVal* version = new RecordVal(software_version);
 	version->Assign(0, major ? major : new Val(-1, TYPE_INT));

src/DFA.cc

@@ -304,6 +304,7 @@ DFA_State_Cache::~DFA_State_Cache()
 		{
 		assert(e->state);
 		delete e->hash;
+		Unref(e->state);
 		delete e;
 		}
 	}
@@ -410,7 +411,10 @@ DFA_Machine::DFA_Machine(NFA_Machine* n, EquivClass* arg_ec)
 		(void) StateSetToDFA_State(state_set, start_state, ec);
 		}
 	else
+		{
 		start_state = 0; // Jam
+		delete ns;
+		}
 	}
 
 DFA_Machine::~DFA_Machine()

src/DebugLogger.cc

@@ -79,6 +79,8 @@ void DebugLogger::EnableStreams(const char* s)
 
 		tok = strtok(0, ",");
 		}
+
+	delete [] tmp;
 	}
 
 void DebugLogger::Log(DebugStream stream, const char* fmt, ...)

src/Expr.cc

@@ -2964,6 +2964,7 @@ Val* IndexExpr::Eval(Frame* f) const
 			if ( v_v1->Size() != v_v2->Size() )
 				{
 				Error("size mismatch, boolean index and vector");
+				Unref(v_result);
 				return 0;
 				}
 
@@ -3803,6 +3804,8 @@ Val* VectorConstructorExpr::InitVal(const BroType* t, Val* aggr) const
 		if ( ! v || ! vec->Assign(i, v) )
 			{
 			Error(fmt("initialization type mismatch at index %d", i), e);
+			if ( ! aggr )
+				Unref(vec);
 			return 0;
 			}
 		}
@@ -5159,6 +5162,7 @@ Val* ListExpr::InitVal(const BroType* t, Val* aggr) const
 		if ( exprs.length() != tl->length() )
 			{
 			Error("index mismatch", t);
+			Unref(v);
 			return 0;
 			}
 

src/Net.cc

@@ -119,6 +119,7 @@ RETSIGTYPE watchdog(int /* signo */)
 					if ( pkt_dumper->IsError() )
 						{
 						reporter->Error("watchdog: can't open watchdog-pkt.pcap for writing\n");
+						delete pkt_dumper;
 						pkt_dumper = 0;
 						}
 					}

src/PktSrc.cc

@@ -624,6 +624,7 @@ SecondaryPath::SecondaryPath()
 		event_list.append(se);
 
 		delete h;
+		Unref(index);
 		}
 	}
 

src/RemoteSerializer.cc

@@ -2699,6 +2699,7 @@ bool RemoteSerializer::ProcessLogCreateWriter()
 	EnumVal* id_val = 0;
 	EnumVal* writer_val = 0;
 	threading::Field** fields = 0;
+	int delete_fields_up_to = -1;
 
 	BinarySerializationFormat fmt;
 	fmt.StartRead(current_args->data, current_args->len);
@@ -2721,8 +2722,11 @@ bool RemoteSerializer::ProcessLogCreateWriter()
 		{
 		fields[i] = new threading::Field;
 		if ( ! fields[i]->Read(&fmt) )
+			{
+			delete_fields_up_to = i + 1;
 			goto error;
 			}
+		}
 
 	fmt.EndRead();
 
@@ -2731,7 +2735,10 @@ bool RemoteSerializer::ProcessLogCreateWriter()
 
 	if ( ! log_mgr->CreateWriter(id_val, writer_val, info, num_fields, fields,
 	                             true, false, true) )
+		{
+		delete_fields_up_to = num_fields;
 		goto error;
+		}
 
 	Unref(id_val);
 	Unref(writer_val);
@@ -2741,7 +2748,12 @@ bool RemoteSerializer::ProcessLogCreateWriter()
 error:
 	Unref(id_val);
 	Unref(writer_val);
+	delete info;
 
+	for ( int i = 0; i < delete_fields_up_to; ++i )
+		delete fields[i];
+
+	delete fields;
 	Error("write error for creating writer");
 	return false;
 	}
@@ -2780,9 +2792,16 @@ bool RemoteSerializer::ProcessLogWrite()
 		for ( int i = 0; i < num_fields; i++ )
 			{
 			vals[i] = new threading::Value;
+
 			if ( ! vals[i]->Read(&fmt) )
+				{
+				for ( int j = 0; j <= i; ++j )
+					delete vals[j];
+
+				delete vals;
 				goto error;
 				}
+			}
 
 		id_val = new EnumVal(id, BifType::Enum::Log::ID);
 		writer_val = new EnumVal(writer, BifType::Enum::Log::Writer);
@@ -4267,6 +4286,8 @@ bool SocketComm::AcceptConnection(int fd)
 	if ( ! peer->io->Init() )
 		{
 		Error(fmt("can't init peer io: %s", peer->io->Error()), false);
+		delete peer->io;
+		delete peer;
 		return false;
 		}
 

src/RuleMatcher.cc

@@ -1226,6 +1226,7 @@ static bool val_to_maskedval(Val* v, maskedvalue_list* append_to,
 
 		default:
 			rules_error("Wrong type of identifier");
+			delete mval;
 			return false;
 	}
 

src/Serializer.cc

@@ -1148,7 +1148,11 @@ Packet* Packet::Unserialize(UnserialInfo* info)
 		UNSERIALIZE(&tv_usec) &&
 		UNSERIALIZE(&len) &&
 		UNSERIALIZE(&p->link_type)) )
+		{
+		delete p;
+		delete hdr;
 		return 0;
+		}
 
 	hdr->ts.tv_sec = tv_sec;
 	hdr->ts.tv_usec = tv_usec;
@@ -1156,12 +1160,18 @@ Packet* Packet::Unserialize(UnserialInfo* info)
 
 	char* tag;
 	if ( ! info->s->Read((char**) &tag, 0, "tag") )
+		{
+		delete p;
+		delete hdr;
 		return 0;
+		}
 
 	char* pkt;
 	int caplen;
 	if ( ! info->s->Read((char**) &pkt, &caplen, "data") )
 		{
+		delete p;
+		delete hdr;
 		delete [] tag;
 		return 0;
 		}

src/Stmt.cc

@@ -788,6 +788,7 @@ bool SwitchStmt::AddCaseLabelMapping(const Val* v, int idx)
 		}
 
 	case_label_map.Insert(hk, new int(idx));
+	delete hk;
 	return true;
 	}
 

src/Var.cc

@@ -416,8 +416,7 @@ void begin_func(ID* id, const char* module_name, function_flavor flavor,
 		if ( arg_id && ! arg_id->IsGlobal() )
 			arg_id->Error("argument name used twice");
 
-		arg_id = install_ID(copy_string(arg_i->id), module_name,
-					false, false);
+		arg_id = install_ID(arg_i->id, module_name, false, false);
 		arg_id->SetType(arg_i->type->Ref());
 		}
 	}

src/analyzer/protocol/dhcp/dhcp-analyzer.pac

@@ -110,6 +110,10 @@ flow DHCP_Flow(is_orig: bool) {
 							       connection()->bro_analyzer()->Conn(),
 							       dhcp_msg_val_->Ref(), host_name);
 				break;
+
+			default:
+				Unref(host_name);
+				break;
 			}
 
 		return true;
@@ -201,6 +205,9 @@ flow DHCP_Flow(is_orig: bool) {
 							    dhcp_msg_val_->Ref(), host_name);
 				break;
 
+			default:
+				Unref(host_name);
+				break;
 			}
 
 		return true;

src/analyzer/protocol/gnutella/Gnutella.cc

@@ -66,6 +66,8 @@ void Gnutella_Analyzer::Done()
 			ConnectionEvent(gnutella_establish, vl);
 		else if ( ! Established () && gnutella_not_establish )
 			ConnectionEvent(gnutella_not_establish, vl);
+		else
+			delete_vals(vl);
 		}
 
 	if ( gnutella_partial_binary_msg )

src/analyzer/protocol/login/RSH.cc

@@ -175,6 +175,9 @@ void Rsh_Analyzer::DeliverStream(int len, const u_char* data, bool orig)
 
 	else if ( rsh_reply )
 		ConnectionEvent(rsh_reply, vl);
+
+	else
+		delete_vals(vl);
 	}
 
 void Rsh_Analyzer::ClientUserName(const char* s)

src/analyzer/protocol/mime/MIME.cc

@@ -831,7 +831,6 @@ int MIME_Entity::ParseContentEncodingField(MIME_Header* h)
 int MIME_Entity::ParseFieldParameters(int len, const char* data)
 	{
 	data_chunk_t attr;
-	BroString* val;
 
 	while ( 1 )
 		{
@@ -864,11 +863,13 @@ int MIME_Entity::ParseFieldParameters(int len, const char* data)
 		data += offset;
 		len -= offset;
 
+		BroString* val = 0;
 		// token or quoted-string
 		offset = MIME_get_value(len, data, val);
 		if ( offset < 0 )
 			{
 			IllegalFormat("value not found in parameter specification");
+			delete val;
 			continue;
 			}
 

src/analyzer/protocol/netbios/NetbiosSSN.cc

@@ -129,15 +129,12 @@ int NetbiosSSN_Interpreter::ParseBroadcast(const u_char* data, int len,
 	data += dstname->Len()+1;
 	len -= dstname->Len();
 
-	if ( smb_session )
-		{
-		smb_session->Deliver(is_query, len, data);
-		return 0;
-		}
-
 	delete srcname;
 	delete dstname;
 
+	if ( smb_session )
+		smb_session->Deliver(is_query, len, data);
+
 	return 0;
 	}
 

src/analyzer/protocol/rpc/NFS.cc

@@ -251,6 +251,7 @@ int NFS_Interp::RPC_BuildReply(RPC_CallInfo* c, BifEnum::rpc_status rpc_status,
 		analyzer->ConnectionEvent(event, vl);
 		}
 
+	Unref(reply);
 	return 1;
 	}
 

src/input/Manager.cc

@@ -416,6 +416,7 @@ bool Manager::CreateEventStream(RecordVal* fval)
 	if ( etype->Flavor() != FUNC_FLAVOR_EVENT )
 		{
 		reporter->Error("stream event is a function, not an event");
+		delete stream;
 		return false;
 		}
 
@@ -424,18 +425,21 @@ bool Manager::CreateEventStream(RecordVal* fval)
 	if ( args->length() < 2 )
 		{
 		reporter->Error("event takes not enough arguments");
+		delete stream;
 		return false;
 		}
 
 	if ( ! same_type((*args)[1], BifType::Enum::Input::Event, 0) )
 		{
 		reporter->Error("events second attribute must be of type Input::Event");
+		delete stream;
 		return false;
 		}
 
 	if ( ! same_type((*args)[0], BifType::Record::Input::EventDescription, 0) )
 		{
 		reporter->Error("events first attribute must be of type Input::EventDescription");
+		delete stream;
 		return false;
 		}
 
@@ -444,6 +448,7 @@ bool Manager::CreateEventStream(RecordVal* fval)
 		if ( args->length() != fields->NumFields() + 2 )
 			{
 			reporter->Error("event has wrong number of arguments");
+			delete stream;
 			return false;
 			}
 
@@ -452,6 +457,7 @@ bool Manager::CreateEventStream(RecordVal* fval)
 			if ( !same_type((*args)[i+2], fields->FieldType(i) ) )
 				{
 				reporter->Error("Incompatible type for event");
+				delete stream;
 				return false;
 				}
 			}
@@ -463,6 +469,7 @@ bool Manager::CreateEventStream(RecordVal* fval)
 		if ( args->length() != 3 )
 			{
 			reporter->Error("event has wrong number of arguments");
+			delete stream;
 			return false;
 			}
 
@@ -475,6 +482,7 @@ bool Manager::CreateEventStream(RecordVal* fval)
 			reporter->Error("Incompatible type '%s':%s for event, which needs type '%s':%s\n",
 					type_name((*args)[2]->Tag()), desc1.Description(),
 					type_name(fields->Tag()), desc2.Description());
+			delete stream;
 			return false;
 			}
 
@@ -493,6 +501,7 @@ bool Manager::CreateEventStream(RecordVal* fval)
 	if ( status )
 		{
 		reporter->Error("Problem unrolling");
+		delete stream;
 		return false;
 		}
 
@@ -560,12 +569,14 @@ bool Manager::CreateTableStream(RecordVal* fval)
 		if ( j >= num )
 			{
 			reporter->Error("Table type has more indexes than index definition");
+			delete stream;
 			return false;
 			}
 
 		if ( ! same_type(idx->FieldType(j), (*tl)[j]) )
 			{
 			reporter->Error("Table type does not match index type");
+			delete stream;
 			return false;
 			}
 		}
@@ -573,6 +584,7 @@ bool Manager::CreateTableStream(RecordVal* fval)
 	if ( num != j )
 		{
 		reporter->Error("Table has less elements than index definition");
+		delete stream;
 		return false;
 		}
 
@@ -589,6 +601,7 @@ bool Manager::CreateTableStream(RecordVal* fval)
 		if ( etype->Flavor() != FUNC_FLAVOR_EVENT )
 			{
 			reporter->Error("stream event is a function, not an event");
+			delete stream;
 			return false;
 			}
 
@@ -597,36 +610,42 @@ bool Manager::CreateTableStream(RecordVal* fval)
 		if ( args->length() != 4 )
 			{
 			reporter->Error("Table event must take 4 arguments");
+			delete stream;
 			return false;
 			}
 
 		if ( ! same_type((*args)[0], BifType::Record::Input::TableDescription, 0) )
 			{
 			reporter->Error("table events first attribute must be of type Input::TableDescription");
+			delete stream;
 			return false;
 			}
 
 		if ( ! same_type((*args)[1], BifType::Enum::Input::Event, 0) )
 			{
 			reporter->Error("table events second attribute must be of type Input::Event");
+			delete stream;
 			return false;
 			}
 
 		if ( ! same_type((*args)[2], idx) )
 			{
 			reporter->Error("table events index attributes do not match");
+			delete stream;
 			return false;
 			}
 
 		if ( want_record->InternalInt() == 1 && ! same_type((*args)[3], val) )
 			{
 			reporter->Error("table events value attributes do not match");
+			delete stream;
 			return false;
 			}
 		else if (  want_record->InternalInt() == 0
 		           && !same_type((*args)[3], val->FieldType(0) ) )
 			{
 			reporter->Error("table events value attribute does not match");
+			delete stream;
 			return false;
 			}
 
@@ -651,6 +670,7 @@ bool Manager::CreateTableStream(RecordVal* fval)
 	if ( status )
 		{
 		reporter->Error("Problem unrolling");
+		delete stream;
 		return false;
 		}
 

src/input/readers/Ascii.cc

@@ -320,6 +320,11 @@ bool Ascii::DoUpdate()
 				{
 				Error(Fmt("Not enough fields in line %s. Found %d fields, want positions %d and %d",
 					  line.c_str(), pos,  (*fit).position, (*fit).secondary_position));
+
+				for ( int i = 0; i < fpos; i++ )
+					delete fields[i];
+
+				delete [] fields;
 				return false;
 				}
 

src/input/readers/Benchmark.cc

@@ -212,6 +212,7 @@ threading::Value* Benchmark::EntryToVal(TypeTag type, TypeTag subtype)
 			if ( newval == 0 )
 				{
 				Error("Error while reading set");
+				delete val;
 				return 0;
 				}
 			lvals[pos] = newval;
@@ -223,6 +224,7 @@ threading::Value* Benchmark::EntryToVal(TypeTag type, TypeTag subtype)
 
 	default:
 		Error(Fmt("unsupported field format %d", type));
+		delete val;
 		return 0;
 	}
 

src/input/readers/SQLite.cc

@@ -148,6 +148,7 @@ Value* SQLite::EntryToVal(sqlite3_stmt *st, const threading::Field *field, int p
 		if ( sqlite3_column_type(st, pos) != SQLITE_INTEGER )
 			{
 			Error("Invalid data type for boolean - expected Integer");
+			delete val;
 			return 0;
 			}
 
@@ -158,6 +159,7 @@ Value* SQLite::EntryToVal(sqlite3_stmt *st, const threading::Field *field, int p
 		else
 			{
 			Error(Fmt("Invalid value for boolean: %d", res));
+			delete val;
 			return 0;
 			}
 		break;
@@ -220,12 +222,14 @@ Value* SQLite::EntryToVal(sqlite3_stmt *st, const threading::Field *field, int p
 		{
 		const char *text = (const char*) sqlite3_column_text(st, pos);
 		string s(text, sqlite3_column_bytes(st, pos));
+		delete val;
 		val = io->ParseValue(s, "", field->type, field->subtype);
 		break;
 		}
 
 	default:
 		Error(Fmt("unsupported field format %d", field->type));
+		delete val;
 		return 0;
 	}
 
@@ -257,6 +261,8 @@ bool SQLite::DoUpdate()
 				if ( mapping[j] != -1 )
 					{
 					Error(Fmt("SQLite statement returns several columns with name %s! Cannot decide which to choose, aborting", name));
+					delete [] mapping;
+					delete [] submapping;
 					return false;
 					}
 
@@ -269,6 +275,8 @@ bool SQLite::DoUpdate()
 				if ( submapping[j] != -1 )
 					{
 					Error(Fmt("SQLite statement returns several columns with name %s! Cannot decide which to choose, aborting", name));
+					delete [] mapping;
+					delete [] submapping;
 					return false;
 					}
 
@@ -282,6 +290,8 @@ bool SQLite::DoUpdate()
 		if ( mapping[i] == -1 )
 			{
 			Error(Fmt("Required field %s not found after SQLite statement", fields[i]->name));
+			delete [] mapping;
+			delete [] submapping;
 			return false;
 			}
 		}
@@ -295,20 +305,28 @@ bool SQLite::DoUpdate()
 			{
 			ofields[j] = EntryToVal(st, fields[j], mapping[j], submapping[j]);
 			if ( ofields[j] == 0 )
+				{
+				for ( int k = 0; k < j; ++k )
+					delete ofields[k];
+
+				delete [] ofields;
+				delete [] mapping;
+				delete [] submapping;
 				return false;
 				}
+			}
 
 		SendEntry(ofields);
 		}
 
+	delete [] mapping;
+	delete [] submapping;
+
 	if ( checkError(errorcode) ) // check the last error code returned by sqlite
 		return false;
 
 	EndCurrentSend();
 
-	delete [] mapping;
-	delete [] submapping;
-
 	if ( checkError(sqlite3_reset(st)) )
 		return false;
 

src/logging/WriterFrontend.cc

@@ -132,6 +132,7 @@ WriterFrontend::~WriterFrontend()
 	Unref(stream);
 	Unref(writer);
 	delete info;
+	delete [] name;
 	}
 
 void WriterFrontend::Stop()

src/main.cc

@@ -222,7 +222,7 @@ void usage()
 
 	fprintf(stderr, "    $BROPATH                       | file search path (%s)\n", bro_path());
 	fprintf(stderr, "    $BROMAGIC                      | libmagic mime magic database search path (%s)\n", bro_magic_path());
-	fprintf(stderr, "    $BRO_PREFIXES                  | prefix list (%s)\n", bro_prefixes());
+	fprintf(stderr, "    $BRO_PREFIXES                  | prefix list (%s)\n", bro_prefixes().c_str());
 	fprintf(stderr, "    $BRO_DNS_FAKE                  | disable DNS lookups (%s)\n", bro_dns_fake());
 	fprintf(stderr, "    $BRO_SEED_FILE                 | file to load seeds from (not set)\n");
 	fprintf(stderr, "    $BRO_LOG_SUFFIX                | ASCII log file extension (.%s)\n", logging::writer::Ascii::LogExt().c_str());

src/probabilistic/CardinalityCounter.cc

@@ -188,5 +188,11 @@ CardinalityCounter* CardinalityCounter::Unserialize(UnserialInfo* info)
 		buckets[i] = (uint8)c;
 		}
 
-	return valid ? c : 0;
+	if ( ! valid )
+		{
+		delete c;
+		c = 0;
+		}
+
+	return c;
 	}

src/probabilistic/Topk.cc

@@ -161,6 +161,7 @@ void TopkVal::Merge(const TopkVal* value, bool doPrune)
 		Element* e = b->elements.front();
 		HashKey* key = GetHash(e->value);
 		elementDict->RemoveEntry(key);
+		delete key;
 		delete e;
 
 		b->elements.pop_front();
@@ -323,6 +324,7 @@ uint64_t TopkVal::GetCount(Val* value) const
 	{
 	HashKey* key = GetHash(value);
 	Element* e = (Element*) elementDict->Lookup(key);
+	delete key;
 
 	if ( e == 0 )
 		{
@@ -330,7 +332,6 @@ uint64_t TopkVal::GetCount(Val* value) const
 		return 0;
 		}
 
-	delete key;
 	return e->parent->count;
 	}
 
@@ -338,6 +339,7 @@ uint64_t TopkVal::GetEpsilon(Val* value) const
 	{
 	HashKey* key = GetHash(value);
 	Element* e = (Element*) elementDict->Lookup(key);
+	delete key;
 
 	if ( e == 0 )
 		{
@@ -345,7 +347,6 @@ uint64_t TopkVal::GetEpsilon(Val* value) const
 		return 0;
 		}
 
-	delete key;
 	return e->epsilon;
 	}
 

src/threading/SerialTypes.h

@@ -126,6 +126,8 @@ struct Value {
 			char* data;
 			int length;
 		} string_val;
+
+		_val() { memset(this, 0, sizeof(_val)); }
 	} val;
 
 	/**

src/util.cc

@@ -902,24 +902,17 @@ const char* bro_magic_path()
 	return path;
 	}
 
-const char* bro_prefixes()
+string bro_prefixes()
 	{
-	int len = 1;	// room for \0
-	loop_over_list(prefixes, i)
-		len += strlen(prefixes[i]) + 1;
-
-	char* p = new char[len];
+	string rval;
 
 	loop_over_list(prefixes, j)
 		if ( j == 0 )
-			strcpy(p, prefixes[j]);
+			rval.append(prefixes[j]);
 		else
-			{
-			strcat(p, ":");
-			strcat(p, prefixes[j]);
-			}
+			rval.append(":").append(prefixes[j]);
 
-	return p;
+	return rval;
 	}
 
 const char* PACKAGE_LOADER = "__load__.bro";

src/util.h

@@ -204,7 +204,7 @@ extern int int_list_cmp(const void* v1, const void* v2);
 
 extern const char* bro_path();
 extern const char* bro_magic_path();
-extern const char* bro_prefixes();
+extern std::string bro_prefixes();
 std::string dot_canon(std::string path, std::string file, std::string prefix = "");
 const char* normalize_path(const char* path);
 void get_script_subpath(const std::string& full_filename, const char** subpath);