From a316878d01adec45049c5a65af92ccc71f24e695 Mon Sep 17 00:00:00 2001 From: Jon Siwek Date: Tue, 17 Sep 2013 16:42:48 -0500 Subject: [PATCH 01/14] Add checks to avoid improper negative values use. --- src/Expr.cc | 14 +++++++-- src/Val.cc | 10 +++++++ src/Val.h | 11 +++++++ src/analyzer/protocol/conn-size/ConnSize.cc | 14 +++++---- src/analyzer/protocol/icmp/ICMP.cc | 6 ++-- src/analyzer/protocol/tcp/TCP.cc | 7 ++--- src/analyzer/protocol/udp/UDP.cc | 8 ++--- .../analyzer/data_event/DataEvent.cc | 9 ++---- src/file_analysis/analyzer/extract/Extract.cc | 3 +- src/input/Manager.cc | 30 +++++++++---------- src/logging/Manager.cc | 29 +++++++++--------- src/util.cc | 10 +++++++ 12 files changed, 89 insertions(+), 62 deletions(-) diff --git a/src/Expr.cc b/src/Expr.cc index e64172675e..f060b98602 100644 --- a/src/Expr.cc +++ b/src/Expr.cc @@ -3137,12 +3137,14 @@ FieldExpr::FieldExpr(Expr* arg_op, const char* arg_field_name) { RecordType* rt = op->Type()->AsRecordType(); field = rt->FieldOffset(field_name); - td = rt->FieldDecl(field); if ( field < 0 ) ExprError("no such field in record"); else + { SetType(rt->FieldType(field)->Ref()); + td = rt->FieldDecl(field); + } } } @@ -3852,7 +3854,15 @@ void FieldAssignExpr::EvalIntoAggregate(const BroType* t, Val* aggr, Frame* f) Val* v = op->Eval(f); if ( v ) - rec->Assign(rt->FieldOffset(field_name.c_str()), v); + { + int idx = rt->FieldOffset(field_name.c_str()); + + if ( idx < 0 ) + reporter->InternalError("Missing record field: %s", + field_name.c_str()); + + rec->Assign(idx, v); + } } int FieldAssignExpr::IsRecordElement(TypeDecl* td) const diff --git a/src/Val.cc b/src/Val.cc index 389e76fe1d..9d1b27b333 100644 --- a/src/Val.cc +++ b/src/Val.cc @@ -2670,6 +2670,16 @@ Val* RecordVal::LookupWithDefault(int field) const return record_type->FieldDefault(field); } +Val* RecordVal::Lookup(const char* field, bool with_default) const + { + int idx = record_type->FieldOffset(field); + + if ( idx < 0 ) + reporter->InternalError("missing record field: %s", field); + + return with_default ? LookupWithDefault(idx) : Lookup(idx); + } + RecordVal* RecordVal::CoerceTo(const RecordType* t, Val* aggr, bool allow_orphaning) const { if ( ! record_promotion_compatible(t->AsRecordType(), Type()->AsRecordType()) ) diff --git a/src/Val.h b/src/Val.h index 019c390699..a7088313d6 100644 --- a/src/Val.h +++ b/src/Val.h @@ -895,6 +895,17 @@ public: Val* Lookup(int field) const; // Does not Ref() value. Val* LookupWithDefault(int field) const; // Does Ref() value. + /** + * Looks up the value of a field by field name. If the field doesn't + * exist in the record type, it's an internal error: abort. + * @param field name of field to lookup. + * @param with_default whether to rely on field's &default attribute when + * the field has yet to be initialized. + * @return the value in field \a field. It is Ref()'d only if + * \a with_default is true. + */ + Val* Lookup(const char* field, bool with_default = false) const; + void Describe(ODesc* d) const; // This is an experiment to associate a BroObj within the diff --git a/src/analyzer/protocol/conn-size/ConnSize.cc b/src/analyzer/protocol/conn-size/ConnSize.cc index b912fe3d2d..ad08c78c4f 100644 --- a/src/analyzer/protocol/conn-size/ConnSize.cc +++ b/src/analyzer/protocol/conn-size/ConnSize.cc @@ -54,17 +54,19 @@ void ConnSize_Analyzer::DeliverPacket(int len, const u_char* data, bool is_orig, void ConnSize_Analyzer::UpdateConnVal(RecordVal *conn_val) { // RecordType *connection_type is decleared in NetVar.h - int orig_endp_idx = connection_type->FieldOffset("orig"); - int resp_endp_idx = connection_type->FieldOffset("resp"); - RecordVal *orig_endp = conn_val->Lookup(orig_endp_idx)->AsRecordVal(); - RecordVal *resp_endp = conn_val->Lookup(resp_endp_idx)->AsRecordVal(); + RecordVal *orig_endp = conn_val->Lookup("orig")->AsRecordVal(); + RecordVal *resp_endp = conn_val->Lookup("resp")->AsRecordVal(); // endpoint is the RecordType from NetVar.h - // TODO: or orig_endp->Type()->AsRecordVal()->FieldOffset() int pktidx = endpoint->FieldOffset("num_pkts"); int bytesidx = endpoint->FieldOffset("num_bytes_ip"); - // TODO: error handling? + if ( pktidx < 0 ) + reporter->InternalError("'endpoint' record missing 'num_pkts' field"); + + if ( bytesidx < 0 ) + reporter->InternalError("'endpoint' record missing 'num_bytes_ip' field"); + orig_endp->Assign(pktidx, new Val(orig_pkts, TYPE_COUNT)); orig_endp->Assign(bytesidx, new Val(orig_bytes, TYPE_COUNT)); resp_endp->Assign(pktidx, new Val(resp_pkts, TYPE_COUNT)); diff --git a/src/analyzer/protocol/icmp/ICMP.cc b/src/analyzer/protocol/icmp/ICMP.cc index 732727d709..a851ebe8ee 100644 --- a/src/analyzer/protocol/icmp/ICMP.cc +++ b/src/analyzer/protocol/icmp/ICMP.cc @@ -440,10 +440,8 @@ void ICMP_Analyzer::Describe(ODesc* d) const void ICMP_Analyzer::UpdateConnVal(RecordVal *conn_val) { - int orig_endp_idx = connection_type->FieldOffset("orig"); - int resp_endp_idx = connection_type->FieldOffset("resp"); - RecordVal *orig_endp = conn_val->Lookup(orig_endp_idx)->AsRecordVal(); - RecordVal *resp_endp = conn_val->Lookup(resp_endp_idx)->AsRecordVal(); + RecordVal *orig_endp = conn_val->Lookup("orig")->AsRecordVal(); + RecordVal *resp_endp = conn_val->Lookup("resp")->AsRecordVal(); UpdateEndpointVal(orig_endp, 1); UpdateEndpointVal(resp_endp, 0); diff --git a/src/analyzer/protocol/tcp/TCP.cc b/src/analyzer/protocol/tcp/TCP.cc index 8f42e0f2b1..c9cd8ed8d1 100644 --- a/src/analyzer/protocol/tcp/TCP.cc +++ b/src/analyzer/protocol/tcp/TCP.cc @@ -1102,11 +1102,8 @@ void TCP_Analyzer::FlipRoles() void TCP_Analyzer::UpdateConnVal(RecordVal *conn_val) { - int orig_endp_idx = connection_type->FieldOffset("orig"); - int resp_endp_idx = connection_type->FieldOffset("resp"); - - RecordVal *orig_endp_val = conn_val->Lookup(orig_endp_idx)->AsRecordVal(); - RecordVal *resp_endp_val = conn_val->Lookup(resp_endp_idx)->AsRecordVal(); + RecordVal *orig_endp_val = conn_val->Lookup("orig")->AsRecordVal(); + RecordVal *resp_endp_val = conn_val->Lookup("resp")->AsRecordVal(); orig_endp_val->Assign(0, new Val(orig->Size(), TYPE_COUNT)); orig_endp_val->Assign(1, new Val(int(orig->state), TYPE_COUNT)); diff --git a/src/analyzer/protocol/udp/UDP.cc b/src/analyzer/protocol/udp/UDP.cc index 3050ea5648..4c26ae5d99 100644 --- a/src/analyzer/protocol/udp/UDP.cc +++ b/src/analyzer/protocol/udp/UDP.cc @@ -170,13 +170,9 @@ void UDP_Analyzer::DeliverPacket(int len, const u_char* data, bool is_orig, void UDP_Analyzer::UpdateConnVal(RecordVal *conn_val) { - int orig_endp_idx = connection_type->FieldOffset("orig"); - int resp_endp_idx = connection_type->FieldOffset("resp"); - RecordVal *orig_endp = conn_val->Lookup(orig_endp_idx)->AsRecordVal(); - RecordVal *resp_endp = conn_val->Lookup(resp_endp_idx)->AsRecordVal(); + RecordVal *orig_endp = conn_val->Lookup("orig")->AsRecordVal(); + RecordVal *resp_endp = conn_val->Lookup("resp")->AsRecordVal(); - orig_endp = conn_val->Lookup(orig_endp_idx)->AsRecordVal(); - resp_endp = conn_val->Lookup(resp_endp_idx)->AsRecordVal(); UpdateEndpointVal(orig_endp, 1); UpdateEndpointVal(resp_endp, 0); diff --git a/src/file_analysis/analyzer/data_event/DataEvent.cc b/src/file_analysis/analyzer/data_event/DataEvent.cc index cf2d7e52ec..a9ffa26bf2 100644 --- a/src/file_analysis/analyzer/data_event/DataEvent.cc +++ b/src/file_analysis/analyzer/data_event/DataEvent.cc @@ -20,13 +20,8 @@ DataEvent::DataEvent(RecordVal* args, File* file, file_analysis::Analyzer* DataEvent::Instantiate(RecordVal* args, File* file) { - using BifType::Record::Files::AnalyzerArgs; - - int chunk_off = AnalyzerArgs->FieldOffset("chunk_event"); - int stream_off = AnalyzerArgs->FieldOffset("stream_event"); - - Val* chunk_val = args->Lookup(chunk_off); - Val* stream_val = args->Lookup(stream_off); + Val* chunk_val = args->Lookup("chunk_event"); + Val* stream_val = args->Lookup("stream_event"); if ( ! chunk_val && ! stream_val ) return 0; diff --git a/src/file_analysis/analyzer/extract/Extract.cc b/src/file_analysis/analyzer/extract/Extract.cc index 504ffd9112..1a3917cd0e 100644 --- a/src/file_analysis/analyzer/extract/Extract.cc +++ b/src/file_analysis/analyzer/extract/Extract.cc @@ -33,8 +33,7 @@ Extract::~Extract() static Val* get_extract_field_val(RecordVal* args, const char* name) { - using BifType::Record::Files::AnalyzerArgs; - Val* rval = args->Lookup(AnalyzerArgs->FieldOffset(name)); + Val* rval = args->Lookup(name); if ( ! rval ) reporter->Error("File extraction analyzer missing arg field: %s", name); diff --git a/src/input/Manager.cc b/src/input/Manager.cc index d838e8cb75..d739a96809 100644 --- a/src/input/Manager.cc +++ b/src/input/Manager.cc @@ -296,7 +296,7 @@ bool Manager::CreateStream(Stream* info, RecordVal* description) return false; } - Val* name_val = description->LookupWithDefault(rtype->FieldOffset("name")); + Val* name_val = description->Lookup("name", true); string name = name_val->AsString()->CheckString(); Unref(name_val); @@ -308,10 +308,10 @@ bool Manager::CreateStream(Stream* info, RecordVal* description) return false; } - EnumVal* reader = description->LookupWithDefault(rtype->FieldOffset("reader"))->AsEnumVal(); + EnumVal* reader = description->Lookup("reader", true)->AsEnumVal(); // get the source ... - Val* sourceval = description->LookupWithDefault(rtype->FieldOffset("source")); + Val* sourceval = description->Lookup("source", true); assert ( sourceval != 0 ); const BroString* bsource = sourceval->AsString(); string source((const char*) bsource->Bytes(), bsource->Len()); @@ -321,7 +321,7 @@ bool Manager::CreateStream(Stream* info, RecordVal* description) rinfo.source = copy_string(source.c_str()); rinfo.name = copy_string(name.c_str()); - EnumVal* mode = description->LookupWithDefault(rtype->FieldOffset("mode"))->AsEnumVal(); + EnumVal* mode = description->Lookup("mode", true)->AsEnumVal(); switch ( mode->InternalInt() ) { case 0: @@ -342,7 +342,7 @@ bool Manager::CreateStream(Stream* info, RecordVal* description) Unref(mode); - Val* config = description->LookupWithDefault(rtype->FieldOffset("config")); + Val* config = description->Lookup("config", true); info->config = config->AsTableVal(); // ref'd by LookupWithDefault { @@ -401,11 +401,11 @@ bool Manager::CreateEventStream(RecordVal* fval) } - RecordType *fields = fval->LookupWithDefault(rtype->FieldOffset("fields"))->AsType()->AsTypeType()->Type()->AsRecordType(); + RecordType *fields = fval->Lookup("fields", true)->AsType()->AsTypeType()->Type()->AsRecordType(); - Val *want_record = fval->LookupWithDefault(rtype->FieldOffset("want_record")); + Val *want_record = fval->Lookup("want_record", true); - Val* event_val = fval->LookupWithDefault(rtype->FieldOffset("ev")); + Val* event_val = fval->Lookup("ev", true); Func* event = event_val->AsFunc(); Unref(event_val); @@ -547,18 +547,18 @@ bool Manager::CreateTableStream(RecordVal* fval) } } - Val* pred = fval->LookupWithDefault(rtype->FieldOffset("pred")); + Val* pred = fval->Lookup("pred", true); - RecordType *idx = fval->LookupWithDefault(rtype->FieldOffset("idx"))->AsType()->AsTypeType()->Type()->AsRecordType(); + RecordType *idx = fval->Lookup("idx", true)->AsType()->AsTypeType()->Type()->AsRecordType(); RecordType *val = 0; - if ( fval->LookupWithDefault(rtype->FieldOffset("val")) != 0 ) + if ( fval->Lookup("val", true) != 0 ) { - val = fval->LookupWithDefault(rtype->FieldOffset("val"))->AsType()->AsTypeType()->Type()->AsRecordType(); + val = fval->Lookup("val", true)->AsType()->AsTypeType()->Type()->AsRecordType(); Unref(val); // The lookupwithdefault in the if-clause ref'ed val. } - TableVal *dst = fval->LookupWithDefault(rtype->FieldOffset("destination"))->AsTableVal(); + TableVal *dst = fval->Lookup("destination", true)->AsTableVal(); // check if index fields match table description int num = idx->NumFields(); @@ -588,9 +588,9 @@ bool Manager::CreateTableStream(RecordVal* fval) return false; } - Val *want_record = fval->LookupWithDefault(rtype->FieldOffset("want_record")); + Val *want_record = fval->Lookup("want_record", true); - Val* event_val = fval->LookupWithDefault(rtype->FieldOffset("ev")); + Val* event_val = fval->Lookup("ev", true); Func* event = event_val ? event_val->AsFunc() : 0; Unref(event_val); diff --git a/src/logging/Manager.cc b/src/logging/Manager.cc index 79e23fb63f..87d3be9f13 100644 --- a/src/logging/Manager.cc +++ b/src/logging/Manager.cc @@ -295,7 +295,7 @@ bool Manager::CreateStream(EnumVal* id, RecordVal* sval) return false; } - RecordType* columns = sval->Lookup(rtype->FieldOffset("columns")) + RecordType* columns = sval->Lookup("columns") ->AsType()->AsTypeType()->Type()->AsRecordType(); bool log_attr_present = false; @@ -322,7 +322,7 @@ bool Manager::CreateStream(EnumVal* id, RecordVal* sval) return false; } - Val* event_val = sval->Lookup(rtype->FieldOffset("ev")); + Val* event_val = sval->Lookup("ev"); Func* event = event_val ? event_val->AsFunc() : 0; if ( event ) @@ -579,19 +579,18 @@ bool Manager::AddFilter(EnumVal* id, RecordVal* fval) return false; // Find the right writer type. - int idx = rtype->FieldOffset("writer"); - EnumVal* writer = fval->LookupWithDefault(idx)->AsEnumVal(); + EnumVal* writer = fval->Lookup("writer", true)->AsEnumVal(); // Create a new Filter instance. - Val* name = fval->LookupWithDefault(rtype->FieldOffset("name")); - Val* pred = fval->LookupWithDefault(rtype->FieldOffset("pred")); - Val* path_func = fval->LookupWithDefault(rtype->FieldOffset("path_func")); - Val* log_local = fval->LookupWithDefault(rtype->FieldOffset("log_local")); - Val* log_remote = fval->LookupWithDefault(rtype->FieldOffset("log_remote")); - Val* interv = fval->LookupWithDefault(rtype->FieldOffset("interv")); - Val* postprocessor = fval->LookupWithDefault(rtype->FieldOffset("postprocessor")); - Val* config = fval->LookupWithDefault(rtype->FieldOffset("config")); + Val* name = fval->Lookup("name", true); + Val* pred = fval->Lookup("pred", true); + Val* path_func = fval->Lookup("path_func", true); + Val* log_local = fval->Lookup("log_local", true); + Val* log_remote = fval->Lookup("log_remote", true); + Val* interv = fval->Lookup("interv", true); + Val* postprocessor = fval->Lookup("postprocessor", true); + Val* config = fval->Lookup("config", true); Filter* filter = new Filter; filter->name = name->AsString()->CheckString(); @@ -616,8 +615,8 @@ bool Manager::AddFilter(EnumVal* id, RecordVal* fval) // Build the list of fields that the filter wants included, including // potentially rolling out fields. - Val* include = fval->Lookup(rtype->FieldOffset("include")); - Val* exclude = fval->Lookup(rtype->FieldOffset("exclude")); + Val* include = fval->Lookup("include"); + Val* exclude = fval->Lookup("exclude"); filter->num_fields = 0; filter->fields = 0; @@ -631,7 +630,7 @@ bool Manager::AddFilter(EnumVal* id, RecordVal* fval) } // Get the path for the filter. - Val* path_val = fval->Lookup(rtype->FieldOffset("path")); + Val* path_val = fval->Lookup("path"); if ( path_val ) { diff --git a/src/util.cc b/src/util.cc index f26b3fb0c2..aeb8fdeef8 100644 --- a/src/util.cc +++ b/src/util.cc @@ -1261,6 +1261,16 @@ void _set_processing_status(const char* status) int fd = open(proc_status_file, O_CREAT | O_WRONLY | O_TRUNC, 0700); + if ( fd < 0 ) + { + char buf[256]; + strerror_r(errno, buf, sizeof(buf)); + reporter->Error("Failed to open process status file '%s': %s", + proc_status_file, buf); + errno = old_errno; + return; + } + int len = strlen(status); while ( len ) { From 79bd81c8da1a845ea22d18eb3012224abb0c6ed4 Mon Sep 17 00:00:00 2001 From: Jon Siwek Date: Thu, 19 Sep 2013 11:39:52 -0500 Subject: [PATCH 02/14] Fix nesting/indent level whitespace mismatch. The EndData() doesn't make sense as part of the condition that implies the state is such that it's not inside data. It might make sense as part of an else block, but it also seems fine to unconditionally EndData() like it currently does. That way unexpected states (mail != 0) are dealt with sooner rather than later. --- src/analyzer/protocol/smtp/SMTP.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/analyzer/protocol/smtp/SMTP.cc b/src/analyzer/protocol/smtp/SMTP.cc index b496e97c69..c3fb21b6a4 100644 --- a/src/analyzer/protocol/smtp/SMTP.cc +++ b/src/analyzer/protocol/smtp/SMTP.cc @@ -599,7 +599,7 @@ void SMTP_Analyzer::UpdateState(const int cmd_code, const int reply_code) case 0: if ( st != SMTP_IN_DATA ) UnexpectedCommand(cmd_code, reply_code); - EndData(); + EndData(); state = SMTP_AFTER_DATA; break; From 1ed210a57247620174326bc530b0fe7a373761b7 Mon Sep 17 00:00:00 2001 From: Jon Siwek Date: Thu, 19 Sep 2013 13:57:51 -0500 Subject: [PATCH 03/14] Set safe umask when creating script profiler tmp files. --- src/Brofiler.cc | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/Brofiler.cc b/src/Brofiler.cc index c9a3505069..777be52217 100644 --- a/src/Brofiler.cc +++ b/src/Brofiler.cc @@ -2,6 +2,7 @@ #include #include #include +#include #include "Brofiler.h" #include "util.h" @@ -54,7 +55,10 @@ bool Brofiler::WriteStats() if ( p && ! p[7] ) { + mode_t old_umask = umask(S_IXUSR | S_IRWXO | S_IRWXG); int fd = mkstemp(bf); + umask(old_umask); + if ( fd == -1 ) { reporter->Error("Failed to generate unique file name from BRO_PROFILER_FILE: %s", bf); From dbbbea312c9a1b133c90fb74b1f37364e9c01ee6 Mon Sep 17 00:00:00 2001 From: Jon Siwek Date: Fri, 20 Sep 2013 11:37:19 -0500 Subject: [PATCH 04/14] Fix DNS_Mgr iterator mismatch. But not really since the global dns_mgr should be equal to "this" while in all the member funcs. Still, better that they always refer to their own instance instead of the global one. --- src/DNS_Mgr.cc | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/DNS_Mgr.cc b/src/DNS_Mgr.cc index e57714a047..5880a02c57 100644 --- a/src/DNS_Mgr.cc +++ b/src/DNS_Mgr.cc @@ -928,7 +928,7 @@ void DNS_Mgr::Save(FILE* f, const HostMap& m) const char* DNS_Mgr::LookupAddrInCache(const IPAddr& addr) { - AddrMap::iterator it = dns_mgr->addr_mappings.find(addr); + AddrMap::iterator it = addr_mappings.find(addr); if ( it == addr_mappings.end() ) return 0; @@ -937,7 +937,7 @@ const char* DNS_Mgr::LookupAddrInCache(const IPAddr& addr) if ( d->Expired() ) { - dns_mgr->addr_mappings.erase(it); + addr_mappings.erase(it); delete d; return 0; } @@ -949,10 +949,10 @@ const char* DNS_Mgr::LookupAddrInCache(const IPAddr& addr) TableVal* DNS_Mgr::LookupNameInCache(string name) { - HostMap::iterator it = dns_mgr->host_mappings.find(name); - if ( it == dns_mgr->host_mappings.end() ) + HostMap::iterator it = host_mappings.find(name); + if ( it == host_mappings.end() ) { - it = dns_mgr->host_mappings.begin(); + it = host_mappings.begin(); return 0; } @@ -964,7 +964,7 @@ TableVal* DNS_Mgr::LookupNameInCache(string name) if ( d4->Expired() || d6->Expired() ) { - dns_mgr->host_mappings.erase(it); + host_mappings.erase(it); delete d4; delete d6; return 0; @@ -979,15 +979,15 @@ TableVal* DNS_Mgr::LookupNameInCache(string name) const char* DNS_Mgr::LookupTextInCache(string name) { - TextMap::iterator it = dns_mgr->text_mappings.find(name); - if ( it == dns_mgr->text_mappings.end() ) + TextMap::iterator it = text_mappings.find(name); + if ( it == text_mappings.end() ) return 0; DNS_Mapping* d = it->second; if ( d->Expired() ) { - dns_mgr->text_mappings.erase(it); + text_mappings.erase(it); delete d; return 0; } From 9033b0384be630b28f00a7b4f58b6f710796d162 Mon Sep 17 00:00:00 2001 From: Jon Siwek Date: Fri, 20 Sep 2013 12:11:10 -0500 Subject: [PATCH 05/14] Fix invalidated iterator usage. --- src/DbgBreakpoint.cc | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/DbgBreakpoint.cc b/src/DbgBreakpoint.cc index 11847fc4dc..f1ad551a00 100644 --- a/src/DbgBreakpoint.cc +++ b/src/DbgBreakpoint.cc @@ -85,10 +85,17 @@ void DbgBreakpoint::RemoveFromGlobalMap() pair p; p = g_debugger_state.breakpoint_map.equal_range(at_stmt); - for ( BPMapType::iterator i = p.first; i != p.second; ++i ) + for ( BPMapType::iterator i = p.first; i != p.second; ) { if ( i->second == this ) + { + BPMapType::iterator next = i; + ++next; g_debugger_state.breakpoint_map.erase(i); + i = next; + } + else + ++i; } } From 4e2e690bffa1dd348016f08c78ebcef53da23720 Mon Sep 17 00:00:00 2001 From: Jon Siwek Date: Fri, 20 Sep 2013 14:21:03 -0500 Subject: [PATCH 06/14] Fix unintentional always-false condition. --- src/logging/Manager.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/logging/Manager.cc b/src/logging/Manager.cc index 87d3be9f13..8d833ddbc6 100644 --- a/src/logging/Manager.cc +++ b/src/logging/Manager.cc @@ -791,7 +791,7 @@ bool Manager::Write(EnumVal* id, RecordVal* columns) if ( ! v ) return false; - if ( ! v->Type()->Tag() == TYPE_STRING ) + if ( v->Type()->Tag() != TYPE_STRING ) { reporter->Error("path_func did not return string"); Unref(v); From 1750e351c4fec8b9f9e51fbca279de25cfa2e171 Mon Sep 17 00:00:00 2001 From: Jon Siwek Date: Fri, 20 Sep 2013 16:36:00 -0500 Subject: [PATCH 07/14] Prevent division/modulo by zero in scripts. Integral/floating-point division/modulo by zero in C++ is undefined behavior, so to prevent such cases in a script from crashing Bro, they're now reported as an error (with script location information) and the event handler in which it occurred returns immediately. --- src/Expr.cc | 43 ++++++++++++++++++++- testing/btest/Baseline/core.div-by-zero/out | 5 +++ testing/btest/core/div-by-zero.bro | 36 +++++++++++++++++ 3 files changed, 82 insertions(+), 2 deletions(-) create mode 100644 testing/btest/Baseline/core.div-by-zero/out create mode 100644 testing/btest/core/div-by-zero.bro diff --git a/src/Expr.cc b/src/Expr.cc index f060b98602..5447999dd9 100644 --- a/src/Expr.cc +++ b/src/Expr.cc @@ -779,9 +779,48 @@ Val* BinaryExpr::Fold(Val* v1, Val* v2) const case EXPR_SUB: DO_FOLD(-); break; case EXPR_REMOVE_FROM: DO_FOLD(-); break; case EXPR_TIMES: DO_FOLD(*); break; - case EXPR_DIVIDE: DO_FOLD(/); break; + case EXPR_DIVIDE: + { + if ( is_integral ) + { + if ( i2 == 0 ) + reporter->ExprRuntimeError(this, "division by zero"); + i3 = i1 / i2; + } + else if ( is_unsigned ) + { + if ( u2 == 0 ) + reporter->ExprRuntimeError(this, "division by zero"); + u3 = u1 / u2; + } + else + { + if ( d2 == 0 ) + reporter->ExprRuntimeError(this, "division by zero"); + d3 = d1 / d2; + } + } + break; + + case EXPR_MOD: + { + if ( is_integral ) + { + if ( i2 == 0 ) + reporter->ExprRuntimeError(this, "modulo by zero"); + i3 = i1 % i2; + } + else if ( is_unsigned ) + { + if ( u2 == 0 ) + reporter->ExprRuntimeError(this, "modulo by zero"); + u3 = u1 % u2; + } + else + Internal("bad type in BinaryExpr::Fold"); + } + break; - case EXPR_MOD: DO_INT_FOLD(%); break; case EXPR_AND: DO_INT_FOLD(&&); break; case EXPR_OR: DO_INT_FOLD(||); break; diff --git a/testing/btest/Baseline/core.div-by-zero/out b/testing/btest/Baseline/core.div-by-zero/out new file mode 100644 index 0000000000..f5524b0cbf --- /dev/null +++ b/testing/btest/Baseline/core.div-by-zero/out @@ -0,0 +1,5 @@ +expression error in /Users/jsiwek/Projects/bro/bro/testing/btest/.tmp/core.div-by-zero/div-by-zero.bro, line 6: division by zero [a / b] +expression error in /Users/jsiwek/Projects/bro/bro/testing/btest/.tmp/core.div-by-zero/div-by-zero.bro, line 11: division by zero [a / b] +expression error in /Users/jsiwek/Projects/bro/bro/testing/btest/.tmp/core.div-by-zero/div-by-zero.bro, line 16: division by zero [a / b] +expression error in /Users/jsiwek/Projects/bro/bro/testing/btest/.tmp/core.div-by-zero/div-by-zero.bro, line 21: modulo by zero [a % b] +expression error in /Users/jsiwek/Projects/bro/bro/testing/btest/.tmp/core.div-by-zero/div-by-zero.bro, line 26: modulo by zero [a % b] diff --git a/testing/btest/core/div-by-zero.bro b/testing/btest/core/div-by-zero.bro new file mode 100644 index 0000000000..d1221638d6 --- /dev/null +++ b/testing/btest/core/div-by-zero.bro @@ -0,0 +1,36 @@ +# @TEST-EXEC: bro -b %INPUT >out 2>&1 +# @TEST-EXEC: TEST_DIFF_CANONIFIER=$SCRIPTS/diff-remove-abspath btest-diff out + +event div_int(a: int, b: int) + { + print a / b; + } + +event div_count(a: count, b: count) + { + print a / b; + } + +event div_double(a: double, b: double) + { + print a / b; + } + +event mod_int(a: int, b: int) + { + print a % b; + } + +event mod_count(a: count, b: count) + { + print a % b; + } + +event bro_init() + { + event div_int(10, 0); + event div_count(10, 0); + event div_double(10.0, 0.0); + event mod_int(10, 0); + event mod_count(10, 0); + } From e4e7c78925e8c3a049f16875fdc33faf4312ec5d Mon Sep 17 00:00:00 2001 From: Jon Siwek Date: Mon, 23 Sep 2013 16:57:31 -0500 Subject: [PATCH 08/14] Remove unused variable assignments, dead code. --- src/CompHash.cc | 1 - src/Debug.cc | 3 +-- src/PersistenceSerializer.cc | 2 -- src/Stmt.cc | 3 +-- src/Val.cc | 6 ++---- 5 files changed, 4 insertions(+), 11 deletions(-) diff --git a/src/CompHash.cc b/src/CompHash.cc index e793a104e0..b2eb08c589 100644 --- a/src/CompHash.cc +++ b/src/CompHash.cc @@ -207,7 +207,6 @@ char* CompositeHash::SingleValHash(int type_check, char* kp0, unsigned int* kp = AlignAndPadType(kp0); VectorVal* vv = v->AsVectorVal(); VectorType* vt = v->Type()->AsVectorType(); - vector* indices = v->AsVector(); *kp = vv->Size(); kp1 = reinterpret_cast(kp+1); for ( unsigned int i = 0; i < vv->Size(); ++i ) diff --git a/src/Debug.cc b/src/Debug.cc index 8cf2e38596..b5aaf91f9f 100644 --- a/src/Debug.cc +++ b/src/Debug.cc @@ -194,8 +194,7 @@ static void parse_function_name(vector& result, return; } - FuncType* ftype; - if ( ! (ftype = id->Type()->AsFuncType()) ) + if ( ! id->Type()->AsFuncType() ) { debug_msg("Function %s not declared.\n", id->Name()); plr.type = plrUnknown; diff --git a/src/PersistenceSerializer.cc b/src/PersistenceSerializer.cc index d9baad05bb..4c6f72d815 100644 --- a/src/PersistenceSerializer.cc +++ b/src/PersistenceSerializer.cc @@ -400,8 +400,6 @@ bool PersistenceSerializer::RunSerialization(SerialStatus* status) while ( (id = status->ids->NextEntry(status->id_cookie)) ) { - ID* g = global_scope()->Lookup(id->Name()); - if ( ! DoIDSerialization(status, id) ) return false; diff --git a/src/Stmt.cc b/src/Stmt.cc index 28f29aba0b..d879c598d2 100644 --- a/src/Stmt.cc +++ b/src/Stmt.cc @@ -1218,9 +1218,8 @@ Val* ForStmt::DoExec(Frame* f, Val* v, stmt_flow_type& flow) const const PDict(TableEntryVal)* loop_vals = tv->AsTable(); HashKey* k; - TableEntryVal* iter_val; IterCookie* c = loop_vals->InitForIteration(); - while ( (iter_val = loop_vals->NextEntry(k, c)) ) + while ( loop_vals->NextEntry(k, c) ) { ListVal* ind_lv = tv->RecoverIndex(k); delete k; diff --git a/src/Val.cc b/src/Val.cc index 9d1b27b333..30c3797253 100644 --- a/src/Val.cc +++ b/src/Val.cc @@ -1656,8 +1656,7 @@ int TableVal::RemoveFrom(Val* val) const IterCookie* c = tbl->InitForIteration(); HashKey* k; - TableEntryVal* v; - while ( (v = tbl->NextEntry(k, c)) ) + while ( tbl->NextEntry(k, c) ) { Val* index = RecoverIndex(k); @@ -1955,8 +1954,7 @@ ListVal* TableVal::ConvertToList(TypeTag t) const IterCookie* c = tbl->InitForIteration(); HashKey* k; - TableEntryVal* v; - while ( (v = tbl->NextEntry(k, c)) ) + while ( tbl->NextEntry(k, c) ) { ListVal* index = table_hash->RecoverVals(k); From daf5d0d098a589dcdc593c21f91fa8c3592710ef Mon Sep 17 00:00:00 2001 From: Jon Siwek Date: Tue, 24 Sep 2013 17:38:22 -0500 Subject: [PATCH 09/14] Improve return value checking and error handling. --- src/Expr.cc | 2 +- src/Obj.cc | 13 ++++++++----- src/PolicyFile.cc | 8 +++++++- src/SerialObj.h | 9 +++++++-- src/Serializer.cc | 7 ++++++- src/Type.cc | 3 ++- src/Val.cc | 19 +++++++++++++++---- src/input/readers/Raw.cc | 25 ++++++++++++++++++++++--- src/input/readers/Raw.h | 1 + src/logging/WriterBackend.h | 11 ----------- src/logging/writers/Ascii.cc | 11 ++++++++++- src/logging/writers/DataSeries.cc | 11 ++++++++++- src/main.cc | 5 ++++- src/util.cc | 9 ++++++++- 14 files changed, 101 insertions(+), 33 deletions(-) diff --git a/src/Expr.cc b/src/Expr.cc index e78a7145e6..e49150f3ac 100644 --- a/src/Expr.cc +++ b/src/Expr.cc @@ -5435,7 +5435,7 @@ TraversalCode ListExpr::Traverse(TraversalCallback* cb) const loop_over_list(exprs, i) { - exprs[i]->Traverse(cb); + tc = exprs[i]->Traverse(cb); HANDLE_TC_EXPR_PRE(tc); } diff --git a/src/Obj.cc b/src/Obj.cc index 0b82695f3d..33aaa8f44e 100644 --- a/src/Obj.cc +++ b/src/Obj.cc @@ -28,11 +28,14 @@ bool Location::DoSerialize(SerialInfo* info) const { DO_SERIALIZE(SER_LOCATION, SerialObj); info->s->WriteOpenTag("Location"); - SERIALIZE(filename); - SERIALIZE(first_line); - SERIALIZE(last_line); - SERIALIZE(first_column); - SERIALIZE(last_column); + + if ( ! (SERIALIZE(filename) && + SERIALIZE(first_line) && + SERIALIZE(last_line) && + SERIALIZE(first_column) && + SERIALIZE(last_column)) ) + return false; + info->s->WriteCloseTag("Location"); return true; } diff --git a/src/PolicyFile.cc b/src/PolicyFile.cc index 9f67ee88a1..369d8c18f4 100644 --- a/src/PolicyFile.cc +++ b/src/PolicyFile.cc @@ -6,6 +6,7 @@ #include #include +#include #include #include @@ -80,7 +81,12 @@ bool LoadPolicyFileText(const char* policy_filename) policy_files.insert(PolicyFileMap::value_type(policy_filename, pf)); struct stat st; - fstat(fileno(f), &st); + if ( fstat(fileno(f), &st) != 0 ) + { + char buf[256]; + strerror_r(errno, buf, sizeof(buf)); + reporter->InternalError("fstat failed on %s: %s", policy_filename, buf); + } pf->lmtime = st.st_mtime; off_t size = st.st_size; diff --git a/src/SerialObj.h b/src/SerialObj.h index c3dc65684c..4794f2bf20 100644 --- a/src/SerialObj.h +++ b/src/SerialObj.h @@ -319,7 +319,8 @@ public: \ if ( has_it ) \ { \ - info->s->Read(&dst, 0, "has_" #dst); \ + if ( ! info->s->Read(&dst, 0, "has_" #dst) ) \ + return false; \ if ( ! dst ) \ return false; \ } \ @@ -339,7 +340,11 @@ public: \ if ( has_it ) \ { \ - info->s->Read(&dst, 0, "has_" #dst); \ + if ( ! info->s->Read(&dst, 0, "has_" #dst) ) \ + { \ + delete del; \ + return 0; \ + } \ if ( ! dst ) \ { \ delete del; \ diff --git a/src/Serializer.cc b/src/Serializer.cc index f7544765fa..bd1be77a2d 100644 --- a/src/Serializer.cc +++ b/src/Serializer.cc @@ -136,7 +136,12 @@ bool Serializer::Serialize(SerialInfo* info, const char* func, val_list* args) Write(network_time, "time"); Write(a, "len"); - loop_over_list(*args, i) (*args)[i]->Serialize(info); + loop_over_list(*args, i) + if ( ! (*args)[i]->Serialize(info) ) + { + Error("failed"); + return false; + } WriteCloseTag("call"); WriteSeparator(); diff --git a/src/Type.cc b/src/Type.cc index 2b9faa8018..706a15aea2 100644 --- a/src/Type.cc +++ b/src/Type.cc @@ -179,7 +179,8 @@ unsigned int BroType::MemoryAllocation() const bool BroType::Serialize(SerialInfo* info) const { // We always send full types (see below). - SERIALIZE(true); + if ( ! SERIALIZE(true) ) + return false; bool ret = SerialObj::Serialize(info); return ret; diff --git a/src/Val.cc b/src/Val.cc index 30c3797253..ffe98ab7bc 100644 --- a/src/Val.cc +++ b/src/Val.cc @@ -81,7 +81,9 @@ Val* Val::Clone() const SerialInfo sinfo(&ss); sinfo.cache = false; - this->Serialize(&sinfo); + if ( ! this->Serialize(&sinfo) ) + return 0; + char* data; uint32 len = form->EndWrite(&data); form->StartRead(data, len); @@ -2326,7 +2328,7 @@ bool TableVal::DoSerialize(SerialInfo* info) const else reporter->InternalError("unknown continuation state"); - HashKey* k; + HashKey* k = 0; int count = 0; assert((!info->cont.ChildSuspended()) || state->v); @@ -2339,12 +2341,21 @@ bool TableVal::DoSerialize(SerialInfo* info) const if ( ! state->c ) { // No next one. - SERIALIZE(false); + if ( ! SERIALIZE(false) ) + { + delete k; + return false; + } + break; } // There's a value coming. - SERIALIZE(true); + if ( ! SERIALIZE(true) ) + { + delete k; + return false; + } if ( state->v->Value() ) state->v->Ref(); diff --git a/src/input/readers/Raw.cc b/src/input/readers/Raw.cc index 7b3f0595b5..70c24eed76 100644 --- a/src/input/readers/Raw.cc +++ b/src/input/readers/Raw.cc @@ -95,6 +95,18 @@ void Raw::ClosePipeEnd(int i) pipes[i] = -1; } +bool Raw::SetFDFlags(int fd, int cmd, int flags) + { + if ( fcntl(fd, cmd, flags) != -1 ) + return true; + + char buf[256]; + strerror_r(errno, buf, sizeof(buf)); + Error(Fmt("failed to set fd flags: %s", buf)); + return false; + } + + bool Raw::LockForkMutex() { int res = pthread_mutex_lock(&fork_mutex); @@ -200,11 +212,13 @@ bool Raw::Execute() ClosePipeEnd(stdout_out); if ( Info().mode == MODE_STREAM ) - fcntl(pipes[stdout_in], F_SETFL, O_NONBLOCK); + if ( ! SetFDFlags(pipes[stdout_in], F_SETFL, O_NONBLOCK) ) + return false; ClosePipeEnd(stdin_in); if ( stdin_towrite ) + { // Ya, just always set this to nonblocking. we do not // want to block on a program receiving data. Note // that there is a small gotcha with it. More data is @@ -213,14 +227,19 @@ bool Raw::Execute() // mode_manual where the first write cannot write // everything, the rest will be stuck in a queue that // is never emptied. - fcntl(pipes[stdin_out], F_SETFL, O_NONBLOCK); + if ( ! SetFDFlags(pipes[stdin_out], F_SETFL, O_NONBLOCK) ) + return false; + } else ClosePipeEnd(stdin_out); ClosePipeEnd(stderr_out); if ( use_stderr ) - fcntl(pipes[stderr_in], F_SETFL, O_NONBLOCK); // true for this too. + { + if ( ! SetFDFlags(pipes[stderr_in], F_SETFL, O_NONBLOCK) ) + return false; + } else ClosePipeEnd(stderr_in); diff --git a/src/input/readers/Raw.h b/src/input/readers/Raw.h index ae6f72524d..c549125174 100644 --- a/src/input/readers/Raw.h +++ b/src/input/readers/Raw.h @@ -31,6 +31,7 @@ protected: private: void ClosePipeEnd(int i); + bool SetFDFlags(int fd, int cmd, int flags); bool LockForkMutex(); bool UnlockForkMutex(); diff --git a/src/logging/WriterBackend.h b/src/logging/WriterBackend.h index b326366b72..f5c74e582c 100644 --- a/src/logging/WriterBackend.h +++ b/src/logging/WriterBackend.h @@ -242,17 +242,6 @@ public: * Note: Exactly one of the two FinishedRotation() methods must be * called by a writer's implementation of DoRotate() once rotation * has finished. - * - * @param new_name The filename of the rotated file. - * - * @param old_name The filename of the original file. - * - * @param open: The timestamp when the original file was opened. - * - * @param close: The timestamp when the origina file was closed. - * - * @param terminating: True if the original rotation request occured - * due to the main Bro process shutting down. */ bool FinishedRotation(); diff --git a/src/logging/writers/Ascii.cc b/src/logging/writers/Ascii.cc index ddb63db36f..1a9cc5c4cd 100644 --- a/src/logging/writers/Ascii.cc +++ b/src/logging/writers/Ascii.cc @@ -261,7 +261,16 @@ bool Ascii::DoRotate(const char* rotated_path, double open, double close, bool t CloseFile(close); string nname = string(rotated_path) + "." + LogExt(); - rename(fname.c_str(), nname.c_str()); + + if ( rename(fname.c_str(), nname.c_str()) != 0 ) + { + char buf[256]; + strerror_r(errno, buf, sizeof(buf)); + Error(Fmt("failed to rename %s to %s: %s", fname.c_str(), + nname.c_str(), buf)); + FinishedRotation(); + return false; + } if ( ! FinishedRotation(nname.c_str(), fname.c_str(), open, close, terminating) ) { diff --git a/src/logging/writers/DataSeries.cc b/src/logging/writers/DataSeries.cc index bc15c6f5b9..087a7061ea 100644 --- a/src/logging/writers/DataSeries.cc +++ b/src/logging/writers/DataSeries.cc @@ -423,7 +423,16 @@ bool DataSeries::DoRotate(const char* rotated_path, double open, double close, b string dsname = string(Info().path) + ".ds"; string nname = string(rotated_path) + ".ds"; - rename(dsname.c_str(), nname.c_str()); + + if ( rename(dsname.c_str(), nname.c_str()) != 0 ) + { + char buf[256]; + strerror_r(errno, buf, sizeof(buf)); + Error(Fmt("failed to rename %s to %s: %s", dsname.c_str(), + nname.c_str(), buf)); + FinishedRotation(); + return false; + } if ( ! FinishedRotation(nname.c_str(), dsname.c_str(), open, close, terminating) ) { diff --git a/src/main.cc b/src/main.cc index 313e1a40b0..0f60a4c70f 100644 --- a/src/main.cc +++ b/src/main.cc @@ -782,7 +782,10 @@ int main(int argc, char** argv) bro_init_magic(&magic_desc_cookie, MAGIC_NONE); bro_init_magic(&magic_mime_cookie, MAGIC_MIME); - sqlite3_initialize(); + int r = sqlite3_initialize(); + + if ( r != SQLITE_OK ) + reporter->Error("Failed to initialize sqlite3: %s", sqlite3_errstr(r)); // FIXME: On systems that don't provide /dev/urandom, OpenSSL doesn't // seed the PRNG. We should do this here (but at least Linux, FreeBSD diff --git a/src/util.cc b/src/util.cc index aeb8fdeef8..a2a21c7bc6 100644 --- a/src/util.cc +++ b/src/util.cc @@ -935,7 +935,7 @@ static const char* check_for_dir(const char* filename, bool load_pkgs) return copy_string(filename); } -FILE* open_file(const char* filename, const char** full_filename, bool load_pkgs) +static FILE* open_file(const char* filename, const char** full_filename, bool load_pkgs) { filename = check_for_dir(filename, load_pkgs); @@ -944,6 +944,13 @@ FILE* open_file(const char* filename, const char** full_filename, bool load_pkgs FILE* f = fopen(filename, "r"); + if ( ! f ) + { + char buf[256]; + strerror_r(errno, buf, sizeof(buf)); + reporter->Error("Failed to open file %s: %s", filename, buf); + } + delete [] filename; return f; From 3c37e818ce1cd78a8de128b6632077f6f95986b8 Mon Sep 17 00:00:00 2001 From: Jon Siwek Date: Wed, 25 Sep 2013 11:16:46 -0500 Subject: [PATCH 10/14] A couple null ptr checks. --- src/BroDoc.cc | 26 ++++++++++++++++++++++---- src/Debug.cc | 2 +- 2 files changed, 23 insertions(+), 5 deletions(-) diff --git a/src/BroDoc.cc b/src/BroDoc.cc index 6953680df0..dd0e8ff37c 100644 --- a/src/BroDoc.cc +++ b/src/BroDoc.cc @@ -504,17 +504,35 @@ static void WritePluginComponents(FILE* f, const plugin::Plugin* p) { switch ( (*it)->Type() ) { case plugin::component::ANALYZER: - WriteAnalyzerComponent(f, - dynamic_cast(*it)); + { + const analyzer::Component* c = + dynamic_cast(*it); + + if ( c ) + WriteAnalyzerComponent(f, c); + else + reporter->InternalError("component type mismatch"); + } break; + case plugin::component::FILE_ANALYZER: - WriteAnalyzerComponent(f, - dynamic_cast(*it)); + { + const file_analysis::Component* c = + dynamic_cast(*it); + + if ( c ) + WriteAnalyzerComponent(f, c); + else + reporter->InternalError("component type mismatch"); + } break; + case plugin::component::READER: reporter->InternalError("docs for READER component unimplemented"); + case plugin::component::WRITER: reporter->InternalError("docs for WRITER component unimplemented"); + default: reporter->InternalError("docs for unknown component unimplemented"); } diff --git a/src/Debug.cc b/src/Debug.cc index b5aaf91f9f..d3cf042ccc 100644 --- a/src/Debug.cc +++ b/src/Debug.cc @@ -721,7 +721,7 @@ static char* get_prompt(bool reset_counter = false) string get_context_description(const Stmt* stmt, const Frame* frame) { ODesc d; - const BroFunc* func = frame->GetFunction(); + const BroFunc* func = frame ? frame->GetFunction() : 0; if ( func ) func->DescribeDebug(&d, frame->GetFuncArgs()); From 2203600e0523c70815d69512a5c00a2e5a0d5b2a Mon Sep 17 00:00:00 2001 From: Jon Siwek Date: Wed, 25 Sep 2013 12:04:07 -0500 Subject: [PATCH 11/14] Fix logic for failed DNS TXT lookups. A failed request should not evict a previous result from the local cache/mapping. --- src/DNS_Mgr.cc | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/src/DNS_Mgr.cc b/src/DNS_Mgr.cc index 5880a02c57..08e8889a10 100644 --- a/src/DNS_Mgr.cc +++ b/src/DNS_Mgr.cc @@ -711,17 +711,19 @@ void DNS_Mgr::AddResult(DNS_Mgr_Request* dr, struct nb_dns_result* r) if ( dr->ReqIsTxt() ) { TextMap::iterator it = text_mappings.find(dr->ReqHost()); + if ( it == text_mappings.end() ) text_mappings[dr->ReqHost()] = new_dm; else { - if ( new_dm->Failed() && prev_dm && prev_dm->Valid() ) - ++keep_prev; - else - { - prev_dm = it->second; - it->second = new_dm; - } + prev_dm = it->second; + it->second = new_dm; + } + + if ( new_dm->Failed() && prev_dm && prev_dm->Valid() ) + { + text_mappings[dr->ReqHost()] = prev_dm; + ++keep_prev; } } else From 4072afb363b24cfa5e783f293d0c3864b905d296 Mon Sep 17 00:00:00 2001 From: Jon Siwek Date: Wed, 25 Sep 2013 12:49:46 -0500 Subject: [PATCH 12/14] Remove dead/unfinished code in unary not expr. The code that looks like it was intended to make it apply to a vector operand couldn't be reached and making it reachable would still require changes to other methods for it to work so just removing for now. --- src/Expr.cc | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/src/Expr.cc b/src/Expr.cc index e49150f3ac..2bd06054d6 100644 --- a/src/Expr.cc +++ b/src/Expr.cc @@ -1129,15 +1129,10 @@ NotExpr::NotExpr(Expr* arg_op) : UnaryExpr(EXPR_NOT, arg_op) return; BroType* t = op->Type(); - if ( IsVector(t->Tag()) ) - t = t->AsVectorType()->YieldType(); - TypeTag bt = t->Tag(); if ( ! IsIntegral(bt) && bt != TYPE_BOOL ) ExprError("requires an integral or boolean operand"); - else if ( IsVector(bt) ) - SetType(new VectorType(base_type(TYPE_BOOL))); else SetType(base_type(TYPE_BOOL)); } @@ -1151,7 +1146,7 @@ Expr* NotExpr::DoSimplify() // !!x == x return ((NotExpr*) op)->Op()->Ref(); - if ( op->IsConst() && ! is_vector(op->ExprVal()) ) + if ( op->IsConst() ) return new ConstExpr(Fold(op->ExprVal())); return this; From 64f3bef96da548bc5e13112dbd090b064d940d03 Mon Sep 17 00:00:00 2001 From: Jon Siwek Date: Wed, 25 Sep 2013 13:37:46 -0500 Subject: [PATCH 13/14] Remove logically dead code. --- src/RemoteSerializer.cc | 2 +- src/Val.cc | 2 +- src/analyzer/protocol/ayiya/ayiya-analyzer.pac | 2 +- src/input/Manager.cc | 1 - 4 files changed, 3 insertions(+), 4 deletions(-) diff --git a/src/RemoteSerializer.cc b/src/RemoteSerializer.cc index bc006b4b1f..e36686b487 100644 --- a/src/RemoteSerializer.cc +++ b/src/RemoteSerializer.cc @@ -1247,7 +1247,7 @@ bool RemoteSerializer::SendCapabilities(Peer* peer) caps |= Peer::PID_64BIT; caps |= Peer::NEW_CACHE_STRATEGY; - return caps ? SendToChild(MSG_CAPS, peer, 3, caps, 0, 0) : true; + return SendToChild(MSG_CAPS, peer, 3, caps, 0, 0); } bool RemoteSerializer::Listen(const IPAddr& ip, uint16 port, bool expect_ssl, diff --git a/src/Val.cc b/src/Val.cc index ffe98ab7bc..bd065f0d8f 100644 --- a/src/Val.cc +++ b/src/Val.cc @@ -2162,7 +2162,7 @@ void TableVal::DoExpire(double t) else if ( v->ExpireAccessTime() + expire_time < t ) { - Val* val = v ? v->Value() : 0; + Val* val = v->Value(); if ( expire_expr ) { diff --git a/src/analyzer/protocol/ayiya/ayiya-analyzer.pac b/src/analyzer/protocol/ayiya/ayiya-analyzer.pac index 7a151453c1..56fcc794bc 100644 --- a/src/analyzer/protocol/ayiya/ayiya-analyzer.pac +++ b/src/analyzer/protocol/ayiya/ayiya-analyzer.pac @@ -79,7 +79,7 @@ flow AYIYA_Flow sessions->DoNextInnerPacket(network_time(), 0, inner, e, ec); - return (result == 0) ? true : false; + return true; %} }; diff --git a/src/input/Manager.cc b/src/input/Manager.cc index d739a96809..f76ab67f0d 100644 --- a/src/input/Manager.cc +++ b/src/input/Manager.cc @@ -1153,7 +1153,6 @@ int Manager::SendEntryTable(Stream* i, const Value* const *vals) { // just quit and delete everything we created. delete idxhash; - delete h; return stream->num_val_fields + stream->num_idx_fields; } From 775ec6795e7d078fff37a882740523f455dfdaeb Mon Sep 17 00:00:00 2001 From: Jon Siwek Date: Fri, 27 Sep 2013 10:13:52 -0500 Subject: [PATCH 14/14] Fix uninitialized (or unused) fields. --- src/Anon.cc | 2 + src/Attr.h | 2 +- src/BPF_Program.cc | 3 +- src/ChunkedIO.cc | 4 +- src/ChunkedIO.h | 5 ++- src/CompHash.cc | 1 + src/Conn.cc | 1 + src/DFA.cc | 1 + src/DNS_Mgr.cc | 12 +++++- src/DbgBreakpoint.cc | 2 + src/Debug.cc | 1 + src/Dict.cc | 4 ++ src/Expr.cc | 11 +++++- src/Expr.h | 5 +-- src/File.h | 1 - src/FlowSrc.cc | 1 - src/FlowSrc.h | 3 -- src/RemoteSerializer.cc | 4 ++ src/Rule.h | 1 + src/RuleMatcher.h | 2 +- src/SerialInfo.h | 1 + src/SerializationFormat.cc | 3 +- src/Serializer.cc | 1 + src/Serializer.h | 1 + src/Sessions.h | 7 +--- src/SmithWaterman.cc | 2 +- src/SmithWaterman.h | 4 +- src/Stats.cc | 1 + src/Stats.h | 12 +----- src/Type.cc | 4 +- src/Val.cc | 1 + src/Val.h | 2 +- src/analyzer/Analyzer.cc | 3 +- src/analyzer/protocol/conn-size/ConnSize.cc | 3 +- src/analyzer/protocol/dns/DNS.cc | 3 +- src/analyzer/protocol/dns/DNS.h | 1 - src/analyzer/protocol/http/HTTP.cc | 1 + src/analyzer/protocol/icmp/ICMP.cc | 5 +-- src/analyzer/protocol/login/Login.cc | 2 +- src/analyzer/protocol/login/NVT.cc | 11 ++---- src/analyzer/protocol/login/RSH.cc | 3 ++ src/analyzer/protocol/mime/MIME.cc | 5 ++- src/analyzer/protocol/mime/MIME.h | 1 - src/analyzer/protocol/netbios/NetbiosSSN.cc | 2 +- src/analyzer/protocol/pia/PIA.cc | 3 +- src/analyzer/protocol/pop3/POP3.cc | 3 ++ src/analyzer/protocol/rpc/RPC.cc | 5 +-- src/analyzer/protocol/smb/SMB.cc | 3 ++ src/analyzer/protocol/tcp/TCP.cc | 3 +- src/analyzer/protocol/tcp/TCP_Endpoint.cc | 3 ++ src/bif_arg.cc | 1 + .../analyzer/unified2/Unified2.h | 1 - src/input/Manager.cc | 37 ++++++++----------- src/input/ReaderBackend.cc | 2 + src/input/readers/Ascii.cc | 3 +- src/input/readers/Benchmark.cc | 2 + src/input/readers/Raw.cc | 3 ++ src/input/readers/SQLite.cc | 4 +- src/logging/WriterFrontend.cc | 3 ++ src/logging/writers/DataSeries.cc | 4 ++ src/logging/writers/SQLite.cc | 6 +-- src/threading/SerialTypes.h | 2 +- 62 files changed, 135 insertions(+), 98 deletions(-) diff --git a/src/Anon.cc b/src/Anon.cc index f58057b2fc..87791501a4 100644 --- a/src/Anon.cc +++ b/src/Anon.cc @@ -147,7 +147,9 @@ void AnonymizeIPAddr_A50::init() special_nodes[0].input = special_nodes[0].output = 0; special_nodes[1].input = special_nodes[1].output = 0xFFFFFFFF; + method = 0; before_anonymization = 1; + new_mapping = 0; } int AnonymizeIPAddr_A50::PreservePrefix(ipaddr32_t input, int num_bits) diff --git a/src/Attr.h b/src/Attr.h index c3e05d4762..7becbb27eb 100644 --- a/src/Attr.h +++ b/src/Attr.h @@ -96,7 +96,7 @@ public: bool operator==(const Attributes& other) const; protected: - Attributes() { type = 0; attrs = 0; } + Attributes() : type(), attrs(), in_record() { } void CheckAttr(Attr* attr); DECLARE_SERIAL(Attributes); diff --git a/src/BPF_Program.cc b/src/BPF_Program.cc index a6d3d80c05..5260429eb0 100644 --- a/src/BPF_Program.cc +++ b/src/BPF_Program.cc @@ -58,9 +58,8 @@ int pcap_compile_nopcap(int snaplen_arg, int linktype_arg, } #endif -BPF_Program::BPF_Program() +BPF_Program::BPF_Program() : m_compiled(), m_program() { - m_compiled = false; } BPF_Program::~BPF_Program() diff --git a/src/ChunkedIO.cc b/src/ChunkedIO.cc index 7e666ee198..22489bbb0c 100644 --- a/src/ChunkedIO.cc +++ b/src/ChunkedIO.cc @@ -14,9 +14,8 @@ #include "NetVar.h" #include "RemoteSerializer.h" -ChunkedIO::ChunkedIO() +ChunkedIO::ChunkedIO() : stats(), tag(), pure() { - pure = false; } void ChunkedIO::Stats(char* buffer, int length) @@ -656,6 +655,7 @@ SSL_CTX* ChunkedIOSSL::ctx; ChunkedIOSSL::ChunkedIOSSL(int arg_socket, bool arg_server) { socket = arg_socket; + last_ret = 0; eof = false; setup = false; server = arg_server; diff --git a/src/ChunkedIO.h b/src/ChunkedIO.h index 56b5656945..6ee79cd3c4 100644 --- a/src/ChunkedIO.h +++ b/src/ChunkedIO.h @@ -292,8 +292,9 @@ private: // Wrapper class around a another ChunkedIO which the (un-)compresses data. class CompressedChunkedIO : public ChunkedIO { public: - CompressedChunkedIO(ChunkedIO* arg_io) - : io(arg_io) {} // takes ownership + CompressedChunkedIO(ChunkedIO* arg_io) // takes ownership + : io(arg_io), zin(), zout(), error(), compress(), uncompress(), + uncompressed_bytes_read(), uncompressed_bytes_written() {} virtual ~CompressedChunkedIO() { delete io; } virtual bool Init(); // does *not* call arg_io->Init() diff --git a/src/CompHash.cc b/src/CompHash.cc index b2eb08c589..5a972f6016 100644 --- a/src/CompHash.cc +++ b/src/CompHash.cc @@ -11,6 +11,7 @@ CompositeHash::CompositeHash(TypeList* composite_type) { type = composite_type; Ref(type); + singleton_tag = TYPE_INTERNAL_ERROR; // If the only element is a record, don't treat it as a // singleton, since it needs to be evaluated specially. diff --git a/src/Conn.cc b/src/Conn.cc index 2e5aa0513e..e221bb20e0 100644 --- a/src/Conn.cc +++ b/src/Conn.cc @@ -141,6 +141,7 @@ Connection::Connection(NetSessions* s, HashKey* k, double t, const ConnID* id, suppress_event = 0; record_contents = record_packets = 1; + record_current_packet = record_current_content = 0; timers_canceled = 0; inactivity_timeout = 0; diff --git a/src/DFA.cc b/src/DFA.cc index 3bdc444f9e..ad9521709e 100644 --- a/src/DFA.cc +++ b/src/DFA.cc @@ -20,6 +20,7 @@ DFA_State::DFA_State(int arg_state_num, const EquivClass* ec, nfa_states = arg_nfa_states; accept = arg_accept; mark = 0; + centry = 0; SymPartition(ec); diff --git a/src/DNS_Mgr.cc b/src/DNS_Mgr.cc index 08e8889a10..c1036accdd 100644 --- a/src/DNS_Mgr.cc +++ b/src/DNS_Mgr.cc @@ -47,8 +47,14 @@ extern int select(int, fd_set *, fd_set *, fd_set *, struct timeval *); class DNS_Mgr_Request { public: DNS_Mgr_Request(const char* h, int af, bool is_txt) - { host = copy_string(h); fam = af; qtype = is_txt ? 16 : 0; } - DNS_Mgr_Request(const IPAddr& a) { addr = a; host = 0; fam = 0; } + : host(copy_string(h)), fam(af), qtype(is_txt ? 16 : 0), addr(), + request_pending() + { } + + DNS_Mgr_Request(const IPAddr& a) + : host(), fam(), qtype(), addr(a), request_pending() + { } + ~DNS_Mgr_Request() { delete [] host; } // Returns nil if this was an address request. @@ -192,6 +198,8 @@ DNS_Mapping::DNS_Mapping(FILE* f) init_failed = 1; req_host = 0; + req_ttl = 0; + creation_time = 0; char buf[512]; diff --git a/src/DbgBreakpoint.cc b/src/DbgBreakpoint.cc index f1ad551a00..9000d89077 100644 --- a/src/DbgBreakpoint.cc +++ b/src/DbgBreakpoint.cc @@ -49,6 +49,8 @@ DbgBreakpoint::DbgBreakpoint() repeat_count = hit_count = 0; description[0] = 0; + source_filename = 0; + source_line = 0; } DbgBreakpoint::~DbgBreakpoint() diff --git a/src/Debug.cc b/src/Debug.cc index d3cf042ccc..f4ac8c2fdf 100644 --- a/src/Debug.cc +++ b/src/Debug.cc @@ -40,6 +40,7 @@ DebuggerState::DebuggerState() next_bp_id = next_watch_id = next_display_id = 1; BreakBeforeNextStmt(false); curr_frame_idx = 0; + already_did_list = false; BreakFromSignal(false); // ### Don't choose this arbitrary size! Extend Frame. diff --git a/src/Dict.cc b/src/Dict.cc index c71cf4c417..cd7792b539 100644 --- a/src/Dict.cc +++ b/src/Dict.cc @@ -64,6 +64,10 @@ Dictionary::Dictionary(dict_order ordering, int initial_size) SetDensityThresh(DEFAULT_DENSITY_THRESH); delete_func = 0; + tbl_next_ind = 0; + + num_buckets2 = num_entries2 = max_num_entries2 = thresh_entries2 = 0; + den_thresh2 = 0; } Dictionary::~Dictionary() diff --git a/src/Expr.cc b/src/Expr.cc index 2bd06054d6..a8aab865ca 100644 --- a/src/Expr.cc +++ b/src/Expr.cc @@ -2472,6 +2472,7 @@ AssignExpr::AssignExpr(Expr* arg_op1, Expr* arg_op2, int arg_is_init, : BinaryExpr(EXPR_ASSIGN, arg_is_init ? arg_op1 : arg_op1->MakeLvalue(), arg_op2) { + val = 0; is_init = arg_is_init; if ( IsError() ) @@ -3345,14 +3346,14 @@ bool HasFieldExpr::DoSerialize(SerialInfo* info) const { DO_SERIALIZE(SER_HAS_FIELD_EXPR, UnaryExpr); - // Serialize the former "bool is_attr" first for backwards compatibility. + // Serialize former "bool is_attr" member first for backwards compatibility. return SERIALIZE(false) && SERIALIZE(field_name) && SERIALIZE(field); } bool HasFieldExpr::DoUnserialize(UnserialInfo* info) { DO_UNSERIALIZE(UnaryExpr); - // Unserialize the former "bool is_attr" first for backwards compatibility. + // Unserialize former "bool is_attr" member for backwards compatibility. bool not_used; return UNSERIALIZE(¬_used) && UNSERIALIZE_STR(&field_name, 0) && UNSERIALIZE(&field); } @@ -3361,6 +3362,8 @@ RecordConstructorExpr::RecordConstructorExpr(ListExpr* constructor_list, BroType* arg_type) : UnaryExpr(EXPR_RECORD_CONSTRUCTOR, constructor_list) { + ctor_type = 0; + if ( IsError() ) return; @@ -3501,6 +3504,8 @@ TableConstructorExpr::TableConstructorExpr(ListExpr* constructor_list, attr_list* arg_attrs, BroType* arg_type) : UnaryExpr(EXPR_TABLE_CONSTRUCTOR, constructor_list) { + attrs = 0; + if ( IsError() ) return; @@ -3625,6 +3630,8 @@ SetConstructorExpr::SetConstructorExpr(ListExpr* constructor_list, attr_list* arg_attrs, BroType* arg_type) : UnaryExpr(EXPR_SET_CONSTRUCTOR, constructor_list) { + attrs = 0; + if ( IsError() ) return; diff --git a/src/Expr.h b/src/Expr.h index ba173feae0..26f20fcbe9 100644 --- a/src/Expr.h +++ b/src/Expr.h @@ -331,12 +331,10 @@ protected: BinaryExpr() { op1 = op2 = 0; } BinaryExpr(BroExprTag arg_tag, Expr* arg_op1, Expr* arg_op2) - : Expr(arg_tag) + : Expr(arg_tag), op1(arg_op1), op2(arg_op2) { if ( ! (arg_op1 && arg_op2) ) return; - op1 = arg_op1; - op2 = arg_op2; if ( op1->IsError() || op2->IsError() ) SetError(); } @@ -749,7 +747,6 @@ protected: DECLARE_SERIAL(HasFieldExpr); - bool is_attr; const char* field_name; int field; }; diff --git a/src/File.h b/src/File.h index a37b7fe0a6..dc56c5a3fe 100644 --- a/src/File.h +++ b/src/File.h @@ -147,7 +147,6 @@ protected: Timer* rotate_timer; double open_time; - bool dont_rotate; // See InstallRotateTimer() bool print_hook; bool raw_output; diff --git a/src/FlowSrc.cc b/src/FlowSrc.cc index f5fb0bd1e6..7a79e9063b 100644 --- a/src/FlowSrc.cc +++ b/src/FlowSrc.cc @@ -64,7 +64,6 @@ void FlowSrc::Close() FlowSocketSrc::~FlowSocketSrc() { - delete [] listenparms; } int FlowSocketSrc::ExtractNextPDU() diff --git a/src/FlowSrc.h b/src/FlowSrc.h index 7b0b14ad15..03dda2761d 100644 --- a/src/FlowSrc.h +++ b/src/FlowSrc.h @@ -67,9 +67,6 @@ public: virtual ~FlowSocketSrc(); int ExtractNextPDU(); - -protected: - char* listenparms; }; class FlowFileSrc : public FlowSrc { diff --git a/src/RemoteSerializer.cc b/src/RemoteSerializer.cc index e36686b487..43a963ab21 100644 --- a/src/RemoteSerializer.cc +++ b/src/RemoteSerializer.cc @@ -541,6 +541,10 @@ RemoteSerializer::RemoteSerializer() in_sync = 0; last_flush = 0; received_logs = 0; + current_id = 0; + current_msgtype = 0; + current_args = 0; + source_peer = 0; } RemoteSerializer::~RemoteSerializer() diff --git a/src/Rule.h b/src/Rule.h index 959008fbf9..e5ae703d39 100644 --- a/src/Rule.h +++ b/src/Rule.h @@ -28,6 +28,7 @@ public: idx = rule_counter++; location = arg_location; active = true; + next = 0; } ~Rule(); diff --git a/src/RuleMatcher.h b/src/RuleMatcher.h index 351c3c30bf..0d7a2fbf7c 100644 --- a/src/RuleMatcher.h +++ b/src/RuleMatcher.h @@ -103,7 +103,7 @@ private: friend class RuleMatcher; struct PatternSet { - PatternSet() {} + PatternSet() : re() {} // If we're above the 'RE_level' (see RuleMatcher), this // expr contains all patterns on this node. If we're on diff --git a/src/SerialInfo.h b/src/SerialInfo.h index aa4c382349..de2d9eeb61 100644 --- a/src/SerialInfo.h +++ b/src/SerialInfo.h @@ -20,6 +20,7 @@ public: SerialInfo(const SerialInfo& info) { + chunk = info.chunk; s = info.s; may_suspend = info.may_suspend; cache = info.cache; diff --git a/src/SerializationFormat.cc b/src/SerializationFormat.cc index 10dd4f29ea..eb8462521e 100644 --- a/src/SerializationFormat.cc +++ b/src/SerializationFormat.cc @@ -6,8 +6,9 @@ #include "Reporter.h" SerializationFormat::SerializationFormat() + : output(), output_size(), output_pos(), input(), input_len(), input_pos(), + bytes_written(), bytes_read() { - output = 0; } SerializationFormat::~SerializationFormat() diff --git a/src/Serializer.cc b/src/Serializer.cc index bd1be77a2d..66ff98ec5d 100644 --- a/src/Serializer.cc +++ b/src/Serializer.cc @@ -1037,6 +1037,7 @@ void ConversionSerializer::GotPacket(Packet* p) } EventPlayer::EventPlayer(const char* file) + : stream_time(), replay_time(), ne_time(), ne_handler(), ne_args() { if ( ! OpenFile(file, true) || fd < 0 ) Error(fmt("event replayer: cannot open %s", file)); diff --git a/src/Serializer.h b/src/Serializer.h index 72e0723880..30095ff4fc 100644 --- a/src/Serializer.h +++ b/src/Serializer.h @@ -403,6 +403,7 @@ public: hdr_size = 0; free = arg_free; tag = arg_tag; + link_type = 0; } ~Packet() diff --git a/src/Sessions.h b/src/Sessions.h index 27b052ba2d..1788541f45 100644 --- a/src/Sessions.h +++ b/src/Sessions.h @@ -55,15 +55,12 @@ struct SessionStats { class TimerMgrExpireTimer : public Timer { public: TimerMgrExpireTimer(double t, TimerMgr* arg_mgr) - : Timer(t, TIMER_TIMERMGR_EXPIRE) - { - mgr = arg_mgr; - } + : Timer(t, TIMER_TIMERMGR_EXPIRE), mgr(arg_mgr) + { } virtual void Dispatch(double t, int is_expire); protected: - double interval; TimerMgr* mgr; }; diff --git a/src/SmithWaterman.cc b/src/SmithWaterman.cc index e051032756..5f2786caa0 100644 --- a/src/SmithWaterman.cc +++ b/src/SmithWaterman.cc @@ -11,7 +11,7 @@ #include "Reporter.h" BroSubstring::BroSubstring(const BroSubstring& bst) -: BroString((const BroString&) bst), _new(bst._new) +: BroString((const BroString&) bst), _num(), _new(bst._new) { for ( BSSAlignVecCIt it = bst._aligns.begin(); it != bst._aligns.end(); ++it ) _aligns.push_back(*it); diff --git a/src/SmithWaterman.h b/src/SmithWaterman.h index 6ea191f5d9..190ecda597 100644 --- a/src/SmithWaterman.h +++ b/src/SmithWaterman.h @@ -42,10 +42,10 @@ public: typedef BSSAlignVec::const_iterator BSSAlignVecCIt; BroSubstring(const string& string) - : BroString(string), _new(false) { } + : BroString(string), _num(), _new(false) { } BroSubstring(const BroString& string) - : BroString(string), _new(false) { } + : BroString(string), _num(), _new(false) { } BroSubstring(const BroSubstring& bst); diff --git a/src/Stats.cc b/src/Stats.cc index 9b839ec672..c4b0ed45b1 100644 --- a/src/Stats.cc +++ b/src/Stats.cc @@ -399,6 +399,7 @@ PacketProfiler::PacketProfiler(unsigned int mode, double freq, last_Utime = last_Stime = last_Rtime = 0.0; last_timestamp = time = 0.0; pkt_cnt = byte_cnt = 0; + last_mem = 0; file->Write("time dt npkts nbytes dRtime dUtime dStime dmem\n"); } diff --git a/src/Stats.h b/src/Stats.h index 8137ad16cf..1bcc2e18dc 100644 --- a/src/Stats.h +++ b/src/Stats.h @@ -29,26 +29,18 @@ public: // The constructor takes some way of identifying the segment. SegmentProfiler(SegmentStatsReporter* arg_reporter, const char* arg_name) + : reporter(arg_reporter), name(arg_name), loc(), initial_rusage() { - reporter = arg_reporter; if ( reporter ) - { - name = arg_name; - loc = 0; Init(); - } } SegmentProfiler(SegmentStatsReporter* arg_reporter, const Location* arg_loc) + : reporter(arg_reporter), name(), loc(arg_loc), initial_rusage() { - reporter = arg_reporter; if ( reporter ) - { - name = 0; - loc = arg_loc; Init(); - } } ~SegmentProfiler() diff --git a/src/Type.cc b/src/Type.cc index 706a15aea2..a6d8b90c6c 100644 --- a/src/Type.cc +++ b/src/Type.cc @@ -1564,10 +1564,8 @@ bool EnumType::DoUnserialize(UnserialInfo* info) } VectorType::VectorType(BroType* element_type) -: BroType(TYPE_VECTOR) + : BroType(TYPE_VECTOR), yield_type(element_type) { - if ( element_type ) - yield_type = element_type; } VectorType::~VectorType() diff --git a/src/Val.cc b/src/Val.cc index bd065f0d8f..d791ff195d 100644 --- a/src/Val.cc +++ b/src/Val.cc @@ -2572,6 +2572,7 @@ unsigned int TableVal::MemoryAllocation() const RecordVal::RecordVal(RecordType* t) : MutableVal(t) { + origin = 0; record_type = t; int n = record_type->NumFields(); val_list* vl = val.val_list_val = new val_list(n); diff --git a/src/Val.h b/src/Val.h index a7088313d6..33bd89c0d5 100644 --- a/src/Val.h +++ b/src/Val.h @@ -466,7 +466,7 @@ public: protected: MutableVal(BroType* t) : Val(t) { props = 0; id = 0; last_modified = SerialObj::ALWAYS; } - MutableVal() { id = 0; last_modified = SerialObj::ALWAYS; } + MutableVal() { props = 0; id = 0; last_modified = SerialObj::ALWAYS; } ~MutableVal(); friend class ID; diff --git a/src/analyzer/Analyzer.cc b/src/analyzer/Analyzer.cc index 7a71c2e0d4..03734f1a22 100644 --- a/src/analyzer/Analyzer.cc +++ b/src/analyzer/Analyzer.cc @@ -20,7 +20,7 @@ public: void Dispatch(double t, int is_expire); protected: - AnalyzerTimer() {} + AnalyzerTimer() : analyzer(), timer(), do_expire() {} void Init(Analyzer* analyzer, analyzer_timer_func timer, int do_expire); @@ -124,6 +124,7 @@ void Analyzer::CtorInit(const Tag& arg_tag, Connection* arg_conn) tag = arg_tag; id = ++id_counter; protocol_confirmed = false; + timers_canceled = false; skip = false; finished = false; removing = false; diff --git a/src/analyzer/protocol/conn-size/ConnSize.cc b/src/analyzer/protocol/conn-size/ConnSize.cc index ad08c78c4f..227a4b1be2 100644 --- a/src/analyzer/protocol/conn-size/ConnSize.cc +++ b/src/analyzer/protocol/conn-size/ConnSize.cc @@ -11,7 +11,8 @@ using namespace analyzer::conn_size; ConnSize_Analyzer::ConnSize_Analyzer(Connection* c) -: Analyzer("CONNSIZE", c) + : Analyzer("CONNSIZE", c), + orig_bytes(), resp_bytes(), orig_pkts(), resp_pkts() { } diff --git a/src/analyzer/protocol/dns/DNS.cc b/src/analyzer/protocol/dns/DNS.cc index 4901df4417..806cb9ae75 100644 --- a/src/analyzer/protocol/dns/DNS.cc +++ b/src/analyzer/protocol/dns/DNS.cc @@ -894,6 +894,7 @@ DNS_MsgInfo::DNS_MsgInfo(DNS_RawMsgHdr* hdr, int arg_is_query) answer_type = DNS_QUESTION; skip_event = 0; + tsig = 0; } DNS_MsgInfo::~DNS_MsgInfo() @@ -1002,7 +1003,7 @@ Contents_DNS::Contents_DNS(Connection* conn, bool orig, interp = arg_interp; msg_buf = 0; - buf_n = msg_size = 0; + buf_n = buf_len = msg_size = 0; state = DNS_LEN_HI; } diff --git a/src/analyzer/protocol/dns/DNS.h b/src/analyzer/protocol/dns/DNS.h index 373250e8f4..af4b8de22f 100644 --- a/src/analyzer/protocol/dns/DNS.h +++ b/src/analyzer/protocol/dns/DNS.h @@ -144,7 +144,6 @@ public: // More values for spesific DNS types. // struct EDNS_ADDITIONAL* edns; - int tsig_init; struct TSIG_DATA* tsig; }; diff --git a/src/analyzer/protocol/http/HTTP.cc b/src/analyzer/protocol/http/HTTP.cc index 71c0052b4b..4bb2385c27 100644 --- a/src/analyzer/protocol/http/HTTP.cc +++ b/src/analyzer/protocol/http/HTTP.cc @@ -534,6 +534,7 @@ HTTP_Message::HTTP_Message(HTTP_Analyzer* arg_analyzer, top_level = new HTTP_Entity(this, 0, expect_body); BeginEntity(top_level); + buffer_offset = buffer_size = 0; data_buffer = 0; total_buffer_size = 0; diff --git a/src/analyzer/protocol/icmp/ICMP.cc b/src/analyzer/protocol/icmp/ICMP.cc index a851ebe8ee..048c6e51ce 100644 --- a/src/analyzer/protocol/icmp/ICMP.cc +++ b/src/analyzer/protocol/icmp/ICMP.cc @@ -17,11 +17,10 @@ using namespace analyzer::icmp; ICMP_Analyzer::ICMP_Analyzer(Connection* c) -: TransportLayerAnalyzer("ICMP", c) + : TransportLayerAnalyzer("ICMP", c), + icmp_conn_val(), type(), code(), request_len(-1), reply_len(-1) { - icmp_conn_val = 0; c->SetInactivityTimeout(icmp_inactivity_timeout); - request_len = reply_len = -1; } void ICMP_Analyzer::Done() diff --git a/src/analyzer/protocol/login/Login.cc b/src/analyzer/protocol/login/Login.cc index 335b855f1d..de2445d81f 100644 --- a/src/analyzer/protocol/login/Login.cc +++ b/src/analyzer/protocol/login/Login.cc @@ -25,7 +25,7 @@ static RE_Matcher* re_login_timeouts; static RE_Matcher* init_RE(ListVal* l); Login_Analyzer::Login_Analyzer(const char* name, Connection* conn) -: tcp::TCP_ApplicationAnalyzer(name, conn) + : tcp::TCP_ApplicationAnalyzer(name, conn), user_text() { state = LOGIN_STATE_AUTHENTICATE; num_user_lines_seen = lines_scanned = 0; diff --git a/src/analyzer/protocol/login/NVT.cc b/src/analyzer/protocol/login/NVT.cc index 62e4d0c050..752ce54df2 100644 --- a/src/analyzer/protocol/login/NVT.cc +++ b/src/analyzer/protocol/login/NVT.cc @@ -364,14 +364,11 @@ void TelnetBinaryOption::InconsistentOption(unsigned int /* type */) NVT_Analyzer::NVT_Analyzer(Connection* conn, bool orig) -: tcp::ContentLine_Analyzer("NVT", conn, orig) + : tcp::ContentLine_Analyzer("NVT", conn, orig), + peer(), pending_IAC(), IAC_pos(), is_suboption(), last_was_IAC(), + binary_mode(), encrypting_mode(), authentication_has_been_accepted(), + auth_name(), options(), num_options() { - peer = 0; - is_suboption = last_was_IAC = pending_IAC = 0; - IAC_pos = 0; - num_options = 0; - authentication_has_been_accepted = encrypting_mode = binary_mode = 0; - auth_name = 0; } NVT_Analyzer::~NVT_Analyzer() diff --git a/src/analyzer/protocol/login/RSH.cc b/src/analyzer/protocol/login/RSH.cc index e70fae1b8c..8aebb89116 100644 --- a/src/analyzer/protocol/login/RSH.cc +++ b/src/analyzer/protocol/login/RSH.cc @@ -22,7 +22,10 @@ Contents_Rsh_Analyzer::Contents_Rsh_Analyzer(Connection* conn, bool orig, if ( orig ) state = save_state = RSH_FIRST_NULL; else + { state = RSH_LINE_MODE; + save_state = RSH_UNKNOWN; + } } Contents_Rsh_Analyzer::~Contents_Rsh_Analyzer() diff --git a/src/analyzer/protocol/mime/MIME.cc b/src/analyzer/protocol/mime/MIME.cc index 5748bece86..c66a0b9be7 100644 --- a/src/analyzer/protocol/mime/MIME.cc +++ b/src/analyzer/protocol/mime/MIME.cc @@ -1257,7 +1257,7 @@ TableVal* MIME_Message::BuildHeaderTable(MIME_HeaderList& hlist) } MIME_Mail::MIME_Mail(analyzer::Analyzer* mail_analyzer, int buf_size) -: MIME_Message(mail_analyzer) + : MIME_Message(mail_analyzer), md5_hash() { analyzer = mail_analyzer; @@ -1280,12 +1280,13 @@ MIME_Mail::MIME_Mail(analyzer::Analyzer* mail_analyzer, int buf_size) if ( mime_content_hash ) { compute_content_hash = 1; - content_hash_length = 0; md5_init(&md5_hash); } else compute_content_hash = 0; + content_hash_length = 0; + top_level = new MIME_Entity(this, 0); // to be changed to MIME_Mail BeginEntity(top_level); } diff --git a/src/analyzer/protocol/mime/MIME.h b/src/analyzer/protocol/mime/MIME.h index 4d7a89f37a..8d83609cc1 100644 --- a/src/analyzer/protocol/mime/MIME.h +++ b/src/analyzer/protocol/mime/MIME.h @@ -247,7 +247,6 @@ protected: int max_chunk_length; int buffer_start; int data_start; - int buffer_offset; int compute_content_hash; int content_hash_length; MD5_CTX md5_hash; diff --git a/src/analyzer/protocol/netbios/NetbiosSSN.cc b/src/analyzer/protocol/netbios/NetbiosSSN.cc index bed906135e..4d6ed8e1f1 100644 --- a/src/analyzer/protocol/netbios/NetbiosSSN.cc +++ b/src/analyzer/protocol/netbios/NetbiosSSN.cc @@ -346,7 +346,7 @@ Contents_NetbiosSSN::Contents_NetbiosSSN(Connection* conn, bool orig, interp = arg_interp; type = flags = msg_size = 0; msg_buf = 0; - buf_n = msg_size = 0; + buf_n = buf_len = msg_size = 0; state = NETBIOS_SSN_TYPE; } diff --git a/src/analyzer/protocol/pia/PIA.cc b/src/analyzer/protocol/pia/PIA.cc index 81c7251820..e94053b8a5 100644 --- a/src/analyzer/protocol/pia/PIA.cc +++ b/src/analyzer/protocol/pia/PIA.cc @@ -7,9 +7,8 @@ using namespace analyzer::pia; PIA::PIA(analyzer::Analyzer* arg_as_analyzer) + : state(INIT), as_analyzer(arg_as_analyzer), conn(), current_packet() { - current_packet.data = 0; - as_analyzer = arg_as_analyzer; } PIA::~PIA() diff --git a/src/analyzer/protocol/pop3/POP3.cc b/src/analyzer/protocol/pop3/POP3.cc index ccbbb8df04..52f9eb8445 100644 --- a/src/analyzer/protocol/pop3/POP3.cc +++ b/src/analyzer/protocol/pop3/POP3.cc @@ -43,6 +43,9 @@ POP3_Analyzer::POP3_Analyzer(Connection* conn) multiLine = false; backOff = false; + lastRequiredCommand = 0; + authLines = 0; + mail = 0; AddSupportAnalyzer(new tcp::ContentLine_Analyzer(conn, true)); diff --git a/src/analyzer/protocol/rpc/RPC.cc b/src/analyzer/protocol/rpc/RPC.cc index ea037c227f..559e629e57 100644 --- a/src/analyzer/protocol/rpc/RPC.cc +++ b/src/analyzer/protocol/rpc/RPC.cc @@ -679,10 +679,9 @@ void Contents_RPC::DeliverStream(int len, const u_char* data, bool orig) RPC_Analyzer::RPC_Analyzer(const char* name, Connection* conn, RPC_Interpreter* arg_interp) -: tcp::TCP_ApplicationAnalyzer(name, conn) + : tcp::TCP_ApplicationAnalyzer(name, conn), + interp(arg_interp), orig_rpc(), resp_rpc() { - interp = arg_interp; - if ( Conn()->ConnTransport() == TRANSPORT_UDP ) ADD_ANALYZER_TIMER(&RPC_Analyzer::ExpireTimer, network_time + rpc_timeout, 1, TIMER_RPC_EXPIRE); diff --git a/src/analyzer/protocol/smb/SMB.cc b/src/analyzer/protocol/smb/SMB.cc index 798e2bfd1f..4393626217 100644 --- a/src/analyzer/protocol/smb/SMB.cc +++ b/src/analyzer/protocol/smb/SMB.cc @@ -100,6 +100,9 @@ static int lookup_IPC_name(BroString* name) SMB_Session::SMB_Session(analyzer::Analyzer* arg_analyzer) { analyzer = arg_analyzer; + req_cmd = 0; + smb_mailslot_prot = false; + smb_pipe_prot = false; dce_rpc_session = 0; init_SMB_command_name(); diff --git a/src/analyzer/protocol/tcp/TCP.cc b/src/analyzer/protocol/tcp/TCP.cc index c9cd8ed8d1..4631833dca 100644 --- a/src/analyzer/protocol/tcp/TCP.cc +++ b/src/analyzer/protocol/tcp/TCP.cc @@ -2043,7 +2043,8 @@ RecordVal* TCPStats_Endpoint::BuildStats() } TCPStats_Analyzer::TCPStats_Analyzer(Connection* c) -: TCP_ApplicationAnalyzer("TCPSTATS", c) + : TCP_ApplicationAnalyzer("TCPSTATS", c), + orig_stats(), resp_stats() { } diff --git a/src/analyzer/protocol/tcp/TCP_Endpoint.cc b/src/analyzer/protocol/tcp/TCP_Endpoint.cc index 3beafdeb8a..144d4598dd 100644 --- a/src/analyzer/protocol/tcp/TCP_Endpoint.cc +++ b/src/analyzer/protocol/tcp/TCP_Endpoint.cc @@ -25,12 +25,15 @@ TCP_Endpoint::TCP_Endpoint(TCP_Analyzer* arg_analyzer, int arg_is_orig) window_scale = 0; window_seq = window_ack_seq = 0; contents_start_seq = 0; + FIN_seq = 0; SYN_cnt = FIN_cnt = RST_cnt = 0; did_close = 0; contents_file = 0; tcp_analyzer = arg_analyzer; is_orig = arg_is_orig; + hist_last_SYN = hist_last_FIN = hist_last_RST = 0; + src_addr = is_orig ? tcp_analyzer->Conn()->RespAddr() : tcp_analyzer->Conn()->OrigAddr(); dst_addr = is_orig ? tcp_analyzer->Conn()->OrigAddr() : diff --git a/src/bif_arg.cc b/src/bif_arg.cc index 64b0cb131a..92e228032b 100644 --- a/src/bif_arg.cc +++ b/src/bif_arg.cc @@ -28,6 +28,7 @@ BuiltinFuncArg::BuiltinFuncArg(const char* arg_name, int arg_type) name = arg_name; type = arg_type; type_str = ""; + attr_str = ""; } BuiltinFuncArg::BuiltinFuncArg(const char* arg_name, const char* arg_type_str, diff --git a/src/file_analysis/analyzer/unified2/Unified2.h b/src/file_analysis/analyzer/unified2/Unified2.h index bd172d2b6b..c2ee9e00da 100644 --- a/src/file_analysis/analyzer/unified2/Unified2.h +++ b/src/file_analysis/analyzer/unified2/Unified2.h @@ -30,7 +30,6 @@ private: binpac::Unified2::Unified2_Analyzer* interp; string filename; - int fd; }; } // namespace file_analysis diff --git a/src/input/Manager.cc b/src/input/Manager.cc index f76ab67f0d..e2562338c8 100644 --- a/src/input/Manager.cc +++ b/src/input/Manager.cc @@ -84,16 +84,16 @@ public: RecordVal* description; - Stream(); virtual ~Stream(); + +protected: + Stream(StreamType t); }; -Manager::Stream::Stream() +Manager::Stream::Stream(StreamType t) + : name(), removed(), stream_type(t), type(), reader(), config(), + description() { - type = 0; - reader = 0; - description = 0; - removed = false; } Manager::Stream::~Stream() @@ -154,24 +154,17 @@ public: ~AnalysisStream(); }; -Manager::TableStream::TableStream() : Manager::Stream::Stream() +Manager::TableStream::TableStream() + : Manager::Stream::Stream(TABLE_STREAM), + num_idx_fields(), num_val_fields(), want_record(), tab(), rtype(), + itype(), currDict(), lastDict(), pred(), event() { - stream_type = TABLE_STREAM; - - tab = 0; - itype = 0; - rtype = 0; - - currDict = 0; - lastDict = 0; - - pred = 0; } -Manager::EventStream::EventStream() : Manager::Stream::Stream() +Manager::EventStream::EventStream() + : Manager::Stream::Stream(EVENT_STREAM), + event(), fields(), num_fields(), want_record() { - fields = 0; - stream_type = EVENT_STREAM; } Manager::EventStream::~EventStream() @@ -204,9 +197,9 @@ Manager::TableStream::~TableStream() } } -Manager::AnalysisStream::AnalysisStream() : Manager::Stream::Stream() +Manager::AnalysisStream::AnalysisStream() + : Manager::Stream::Stream(ANALYSIS_STREAM), file_id() { - stream_type = ANALYSIS_STREAM; } Manager::AnalysisStream::~AnalysisStream() diff --git a/src/input/ReaderBackend.cc b/src/input/ReaderBackend.cc index 0aaadc3cdc..abf369dd54 100644 --- a/src/input/ReaderBackend.cc +++ b/src/input/ReaderBackend.cc @@ -163,6 +163,8 @@ ReaderBackend::ReaderBackend(ReaderFrontend* arg_frontend) : MsgThread() disabled = true; // disabled will be set correcty in init. frontend = arg_frontend; info = new ReaderInfo(frontend->Info()); + num_fields = 0; + fields = 0; SetName(frontend->Name()); } diff --git a/src/input/readers/Ascii.cc b/src/input/readers/Ascii.cc index 687d37c96f..8f61e17fc0 100644 --- a/src/input/readers/Ascii.cc +++ b/src/input/readers/Ascii.cc @@ -18,7 +18,7 @@ using threading::Value; using threading::Field; FieldMapping::FieldMapping(const string& arg_name, const TypeTag& arg_type, int arg_position) - : name(arg_name), type(arg_type) + : name(arg_name), type(arg_type), subtype(TYPE_ERROR) { position = arg_position; secondary_position = -1; @@ -49,6 +49,7 @@ FieldMapping FieldMapping::subType() Ascii::Ascii(ReaderFrontend *frontend) : ReaderBackend(frontend) { file = 0; + mtime = 0; separator.assign( (const char*) BifConst::InputAscii::separator->Bytes(), BifConst::InputAscii::separator->Len()); diff --git a/src/input/readers/Benchmark.cc b/src/input/readers/Benchmark.cc index 3e53dfb35f..ec6b382ebb 100644 --- a/src/input/readers/Benchmark.cc +++ b/src/input/readers/Benchmark.cc @@ -18,6 +18,7 @@ using threading::Field; Benchmark::Benchmark(ReaderFrontend *frontend) : ReaderBackend(frontend) { + num_lines = 0; multiplication_factor = double(BifConst::InputBenchmark::factor); autospread = double(BifConst::InputBenchmark::autospread); spread = int(BifConst::InputBenchmark::spread); @@ -25,6 +26,7 @@ Benchmark::Benchmark(ReaderFrontend *frontend) : ReaderBackend(frontend) autospread_time = 0; stopspreadat = int(BifConst::InputBenchmark::stopspreadat); timedspread = double(BifConst::InputBenchmark::timedspread); + heartbeatstarttime = 0; heartbeat_interval = double(BifConst::Threading::heartbeat_interval); ascii = new AsciiFormatter(this, AsciiFormatter::SeparatorInfo()); diff --git a/src/input/readers/Raw.cc b/src/input/readers/Raw.cc index 70c24eed76..1947a907cb 100644 --- a/src/input/readers/Raw.cc +++ b/src/input/readers/Raw.cc @@ -34,6 +34,9 @@ Raw::Raw(ReaderFrontend *frontend) : ReaderBackend(frontend) { file = 0; stderrfile = 0; + execute = false; + firstrun = true; + mtime = 0; forcekill = false; separator.assign( (const char*) BifConst::InputRaw::record_separator->Bytes(), BifConst::InputRaw::record_separator->Len()); diff --git a/src/input/readers/SQLite.cc b/src/input/readers/SQLite.cc index 84b57e2509..abb155e3bc 100644 --- a/src/input/readers/SQLite.cc +++ b/src/input/readers/SQLite.cc @@ -17,7 +17,9 @@ using namespace input::reader; using threading::Value; using threading::Field; -SQLite::SQLite(ReaderFrontend *frontend) : ReaderBackend(frontend) +SQLite::SQLite(ReaderFrontend *frontend) + : ReaderBackend(frontend), + fields(), num_fields(), mode(), started(), query(), db(), st() { set_separator.assign( (const char*) BifConst::LogSQLite::set_separator->Bytes(), diff --git a/src/logging/WriterFrontend.cc b/src/logging/WriterFrontend.cc index 1e254bfc83..09490ce3d1 100644 --- a/src/logging/WriterFrontend.cc +++ b/src/logging/WriterFrontend.cc @@ -112,6 +112,9 @@ WriterFrontend::WriterFrontend(const WriterBackend::WriterInfo& arg_info, EnumVa write_buffer_pos = 0; info = new WriterBackend::WriterInfo(arg_info); + num_fields = 0; + fields = 0; + const char* w = arg_writer->Type()->AsEnumType()->Lookup(arg_writer->InternalInt()); name = copy_string(fmt("%s/%s", arg_info.path, w)); diff --git a/src/logging/writers/DataSeries.cc b/src/logging/writers/DataSeries.cc index 087a7061ea..1073e31a08 100644 --- a/src/logging/writers/DataSeries.cc +++ b/src/logging/writers/DataSeries.cc @@ -233,6 +233,10 @@ DataSeries::DataSeries(WriterFrontend* frontend) : WriterBackend(frontend) ds_set_separator = ","; ascii = new AsciiFormatter(this, AsciiFormatter::SeparatorInfo()); + + compress_type = Extent::compress_none; + log_file = 0; + log_output = 0; } DataSeries::~DataSeries() diff --git a/src/logging/writers/SQLite.cc b/src/logging/writers/SQLite.cc index 81c52fe198..c1614f1aed 100644 --- a/src/logging/writers/SQLite.cc +++ b/src/logging/writers/SQLite.cc @@ -16,7 +16,9 @@ using namespace writer; using threading::Value; using threading::Field; -SQLite::SQLite(WriterFrontend* frontend) : WriterBackend(frontend) +SQLite::SQLite(WriterFrontend* frontend) + : WriterBackend(frontend), + fields(), num_fields(), db(), st() { set_separator.assign( (const char*) BifConst::LogSQLite::set_separator->Bytes(), @@ -33,9 +35,7 @@ SQLite::SQLite(WriterFrontend* frontend) : WriterBackend(frontend) BifConst::LogSQLite::empty_field->Len() ); - db = 0; io = new AsciiFormatter(this, AsciiFormatter::SeparatorInfo(set_separator, unset_field, empty_field)); - st = 0; } SQLite::~SQLite() diff --git a/src/threading/SerialTypes.h b/src/threading/SerialTypes.h index 8a4f46a15c..e056f6c170 100644 --- a/src/threading/SerialTypes.h +++ b/src/threading/SerialTypes.h @@ -81,7 +81,7 @@ private: friend class ::RemoteSerializer; // Force usage of constructor above. - Field() {}; + Field() {} }; /**