Fixing a big pile of Coverity issues

1469562/1469558: Uninitialized fields in Func constructor
1469571/1469566: Null pointer dereference in Trigger::Init()
1469568: Uninitialized fields in CounterVector constructor
1469570: Uncaught exception in plugin manager
1469569: Resource leak in script_opt::Stmt
1469561/1469561: Uninitialized fields in ZBody constructor
1469559: Uninitialized fields in logging::Manager
1469563: Resource leak in ZAMCompiler::CompileDel
1469549/1469553/1469556: Context not fully initialized in HashVals
1469548: Remove dead code from IPAddr
1469551/1469554: Handle iosource_mgr registration failure in broker::Manager
1469552/1469572: Resource leaks in input::Manager
This commit is contained in:
Tim Wojtulewicz 2022-02-09 10:44:10 -07:00
parent 41abf8f422
commit a117c81d85
13 changed files with 93 additions and 70 deletions

View file

@ -139,8 +139,8 @@ protected:
std::vector<Body> bodies; std::vector<Body> bodies;
detail::ScopePtr scope; detail::ScopePtr scope;
Kind kind; Kind kind = SCRIPT_FUNC;
uint32_t unique_id; uint32_t unique_id = 0;
FuncTypePtr type; FuncTypePtr type;
std::string name; std::string name;
static inline std::vector<FuncPtr> unique_ids; static inline std::vector<FuncPtr> unique_ids;
@ -298,7 +298,7 @@ protected:
virtual void SetCaptures(Frame* f); virtual void SetCaptures(Frame* f);
private: private:
size_t frame_size; size_t frame_size = 0;
// List of the outer IDs used in the function. // List of the outer IDs used in the function.
IDPList outer_ids; IDPList outer_ids;
@ -369,8 +369,8 @@ struct function_ingredients
IDPtr id; IDPtr id;
StmtPtr body; StmtPtr body;
std::vector<IDPtr> inits; std::vector<IDPtr> inits;
int frame_size; int frame_size = 0;
int priority; int priority = 0;
ScopePtr scope; ScopePtr scope;
}; };

View file

@ -504,7 +504,6 @@ inline void IPAddr::ConvertToThreadingValue(threading::Value::addr_t* v) const
switch ( v->family ) switch ( v->family )
{ {
case IPv4: case IPv4:
CopyIPv4(&v->in.in4); CopyIPv4(&v->in.in4);
return; return;
@ -512,9 +511,6 @@ inline void IPAddr::ConvertToThreadingValue(threading::Value::addr_t* v) const
case IPv6: case IPv6:
CopyIPv6(&v->in.in6); CopyIPv6(&v->in.in6);
return; return;
// Can't be reached.
abort();
} }
} }

View file

@ -215,7 +215,10 @@ HashVal::HashVal(OpaqueTypePtr t) : OpaqueVal(std::move(t))
valid = false; valid = false;
} }
MD5Val::MD5Val() : HashVal(md5_type) { } MD5Val::MD5Val() : HashVal(md5_type)
{
memset(&ctx, 0, sizeof(ctx));
}
MD5Val::~MD5Val() { } MD5Val::~MD5Val() { }
@ -324,7 +327,10 @@ bool MD5Val::DoUnserialize(const broker::data& data)
return true; return true;
} }
SHA1Val::SHA1Val() : HashVal(sha1_type) { } SHA1Val::SHA1Val() : HashVal(sha1_type)
{
memset(&ctx, 0, sizeof(ctx));
}
SHA1Val::~SHA1Val() { } SHA1Val::~SHA1Val() { }
@ -414,7 +420,10 @@ bool SHA1Val::DoUnserialize(const broker::data& data)
return true; return true;
} }
SHA256Val::SHA256Val() : HashVal(sha256_type) { } SHA256Val::SHA256Val() : HashVal(sha256_type)
{
memset(&ctx, 0, sizeof(ctx));
}
SHA256Val::~SHA256Val() { } SHA256Val::~SHA256Val() { }

View file

@ -166,7 +166,7 @@ void Trigger::Init(ExprPtr arg_cond, StmtPtr arg_body, StmtPtr arg_timeout_stmts
DBG_LOG(DBG_NOTIFIERS, "%s: instantiating", Name()); DBG_LOG(DBG_NOTIFIERS, "%s: instantiating", Name());
if ( is_return ) if ( is_return && frame && arg_frame )
{ {
Trigger* parent = frame->GetTrigger(); Trigger* parent = frame->GetTrigger();
if ( ! parent ) if ( ! parent )

View file

@ -1811,7 +1811,10 @@ detail::StoreHandleVal* Manager::MakeMaster(const string& name, broker::backend
Ref(handle); Ref(handle);
data_stores.emplace(name, handle); data_stores.emplace(name, handle);
iosource_mgr->RegisterFd(handle->proxy.mailbox().descriptor(), this); if ( ! iosource_mgr->RegisterFd(handle->proxy.mailbox().descriptor(), this) )
reporter->FatalError(
"Failed to register broker master mailbox descriptor with iosource_mgr");
PrepareForwarding(name); PrepareForwarding(name);
if ( ! bstate->endpoint.use_real_time() ) if ( ! bstate->endpoint.use_real_time() )
@ -1916,7 +1919,9 @@ detail::StoreHandleVal* Manager::MakeClone(const string& name, double resync_int
Ref(handle); Ref(handle);
data_stores.emplace(name, handle); data_stores.emplace(name, handle);
iosource_mgr->RegisterFd(handle->proxy.mailbox().descriptor(), this); if ( ! iosource_mgr->RegisterFd(handle->proxy.mailbox().descriptor(), this) )
reporter->FatalError(
"Failed to register broker clone mailbox descriptor with iosource_mgr");
PrepareForwarding(name); PrepareForwarding(name);
return handle; return handle;
} }

View file

@ -1473,8 +1473,9 @@ void Manager::SendEndOfData(const Stream* i)
#ifdef DEBUG #ifdef DEBUG
DBG_LOG(DBG_INPUT, "SendEndOfData for stream %s", i->name.c_str()); DBG_LOG(DBG_INPUT, "SendEndOfData for stream %s", i->name.c_str());
#endif #endif
SendEvent(end_of_data, 2, new StringVal(i->name.c_str()), auto name = make_intrusive<StringVal>(i->name.c_str());
new StringVal(i->reader->Info().source)); auto source = make_intrusive<StringVal>(i->reader->Info().source);
SendEvent(end_of_data, 2, name->Ref(), source->Ref());
if ( i->stream_type == ANALYSIS_STREAM ) if ( i->stream_type == ANALYSIS_STREAM )
file_mgr->EndOfFile(static_cast<const AnalysisStream*>(i)->file_id); file_mgr->EndOfFile(static_cast<const AnalysisStream*>(i)->file_id);
@ -2560,8 +2561,8 @@ void Manager::ErrorHandler(const Stream* i, ErrorType et, bool reporter_send, co
__builtin_unreachable(); __builtin_unreachable();
} }
auto* message = new StringVal(buf); auto message = make_intrusive<StringVal>(buf);
SendEvent(i->error_event, 3, i->description->Ref(), message, ev.release()); SendEvent(i->error_event, 3, i->description->Ref(), message->Ref(), ev.release());
} }
if ( reporter_send ) if ( reporter_send )

View file

@ -31,27 +31,27 @@ namespace zeek::logging
struct Manager::Filter struct Manager::Filter
{ {
Val* fval; Val* fval = nullptr;
string name; string name;
EnumVal* id; EnumVal* id = nullptr;
Func* policy; Func* policy = nullptr;
Func* path_func; Func* path_func = nullptr;
string path; string path;
Val* path_val; Val* path_val = nullptr;
EnumVal* writer; EnumVal* writer = nullptr;
TableVal* config; TableVal* config = nullptr;
TableVal* field_name_map; TableVal* field_name_map = nullptr;
string scope_sep; string scope_sep;
string ext_prefix; string ext_prefix;
Func* ext_func; Func* ext_func = nullptr;
int num_ext_fields; int num_ext_fields = 0;
bool local; bool local = false;
bool remote; bool remote = false;
double interval; double interval = 0.0;
Func* postprocessor; Func* postprocessor = nullptr;
int num_fields; int num_fields = 0;
threading::Field** fields; threading::Field** fields = nullptr;
// Vector indexed by field number. Each element is a list of record // Vector indexed by field number. Each element is a list of record
// indices defining a path leading to the value across potential // indices defining a path leading to the value across potential
@ -63,26 +63,26 @@ struct Manager::Filter
struct Manager::WriterInfo struct Manager::WriterInfo
{ {
EnumVal* type; EnumVal* type = nullptr;
double open_time; double open_time = 0.0;
detail::Timer* rotation_timer; detail::Timer* rotation_timer = nullptr;
double interval; double interval = 0.0;
Func* postprocessor; Func* postprocessor = nullptr;
WriterFrontend* writer; WriterFrontend* writer = nullptr;
WriterBackend::WriterInfo* info; WriterBackend::WriterInfo* info = nullptr;
bool from_remote; bool from_remote = false;
bool hook_initialized; bool hook_initialized = false;
string instantiating_filter; string instantiating_filter;
}; };
struct Manager::Stream struct Manager::Stream
{ {
EnumVal* id; EnumVal* id = nullptr;
bool enabled; bool enabled = false;
string name; string name;
RecordType* columns; RecordType* columns = nullptr;
EventHandlerPtr event; EventHandlerPtr event;
Func* policy; Func* policy = nullptr;
list<Filter*> filters; list<Filter*> filters;
using WriterPathPair = pair<int, string>; using WriterPathPair = pair<int, string>;

View file

@ -423,16 +423,26 @@ void Manager::ExtendZeekPathForPlugins()
if ( p->DynamicPlugin() || p->Name().empty() ) if ( p->DynamicPlugin() || p->Name().empty() )
continue; continue;
string canon = std::regex_replace(p->Name(), std::regex("::"), "_"); try
string dir = "builtin-plugins/" + canon; {
string canon = std::regex_replace(p->Name(), std::regex("::"), "_");
string dir = "builtin-plugins/" + canon;
// Use find_file to find the directory in the path. // Use find_file to find the directory in the path.
string script_dir = util::find_file(dir, util::zeek_path()); string script_dir = util::find_file(dir, util::zeek_path());
if ( script_dir.empty() || ! util::is_dir(script_dir) ) if ( script_dir.empty() || ! util::is_dir(script_dir) )
continue; continue;
DBG_LOG(DBG_PLUGINS, " Adding %s to ZEEKPATH", script_dir.c_str()); DBG_LOG(DBG_PLUGINS, " Adding %s to ZEEKPATH", script_dir.c_str());
path_additions.push_back(script_dir); path_additions.push_back(script_dir);
}
catch ( const std::regex_error& e )
{
// This really shouldn't ever happen, but we do need to catch the exception.
// Report a fatal error because something is wrong if this occurs.
reporter->FatalError("Failed to replace colons in plugin name %s: %s",
p->Name().c_str(), e.what());
}
} }
for ( const auto& plugin_path : path_additions ) for ( const auto& plugin_path : path_additions )

View file

@ -150,13 +150,13 @@ public:
protected: protected:
friend CounterVector operator|(const CounterVector& x, const CounterVector& y); friend CounterVector operator|(const CounterVector& x, const CounterVector& y);
CounterVector() { } CounterVector() = default;
private: private:
CounterVector& operator=(const CounterVector&); // Disable. CounterVector& operator=(const CounterVector&); // Disable.
BitVector* bits; BitVector* bits = nullptr;
size_t width; size_t width = 0;
}; };
} // namespace zeek::probabilistic::detail } // namespace zeek::probabilistic::detail

View file

@ -171,12 +171,12 @@ private:
void Typify(TypePtr t); void Typify(TypePtr t);
TypePtr type; TypePtr type;
zeek::detail::CompositeHash* hash; zeek::detail::CompositeHash* hash = nullptr;
std::list<Bucket*> buckets; std::list<Bucket*> buckets;
PDict<Element>* elementDict; PDict<Element>* elementDict = nullptr;
uint64_t size; // how many elements are we tracking? uint64_t size = 0; // how many elements are we tracking?
uint64_t numElements; // how many elements do we have at the moment uint64_t numElements = 0; // how many elements do we have at the moment
bool pruned; // was this data structure pruned? bool pruned = false; // was this data structure pruned?
}; };
} // namespace zeek::probabilistic::detail } // namespace zeek::probabilistic::detail

View file

@ -728,7 +728,7 @@ bool StmtList::IsReduced(Reducer* c) const
StmtPtr StmtList::DoReduce(Reducer* c) StmtPtr StmtList::DoReduce(Reducer* c)
{ {
StmtPList* f_stmts = new StmtPList; StmtPList* f_stmts = new StmtPList{};
bool did_change = false; bool did_change = false;
int n = Stmts().length(); int n = Stmts().length();
@ -749,7 +749,10 @@ StmtPtr StmtList::DoReduce(Reducer* c)
} }
if ( f_stmts->length() == 0 ) if ( f_stmts->length() == 0 )
{
delete f_stmts;
return TransformMe(make_intrusive<NullStmt>(), c); return TransformMe(make_intrusive<NullStmt>(), c);
}
if ( f_stmts->length() == 1 ) if ( f_stmts->length() == 1 )
return (*f_stmts)[0]->Reduce(c); return (*f_stmts)[0]->Reduce(c);

View file

@ -711,9 +711,8 @@ const ZAMStmt ZAMCompiler::CompileDel(const DelStmt* ds)
if ( index_list->Tag() != EXPR_LIST ) if ( index_list->Tag() != EXPR_LIST )
reporter->InternalError("non-list in \"delete\""); reporter->InternalError("non-list in \"delete\"");
auto internal_ind = BuildVals(index_list->AsListExprPtr()); auto internal_ind = std::unique_ptr<OpaqueVals>(BuildVals(index_list->AsListExprPtr()));
return DelTableVO(aggr, internal_ind.get());
return DelTableVO(aggr, internal_ind);
} }
const ZAMStmt ZAMCompiler::CompileWhile(const WhileStmt* ws) const ZAMStmt ZAMCompiler::CompileWhile(const WhileStmt* ws)

View file

@ -70,7 +70,7 @@ protected:
TraversalCode Traverse(TraversalCallback* cb) const override; TraversalCode Traverse(TraversalCallback* cb) const override;
private: private:
const char* func_name; const char* func_name = nullptr;
const ZInst* insts = nullptr; const ZInst* insts = nullptr;
unsigned int ninst; unsigned int ninst;
@ -109,7 +109,7 @@ private:
// const method. // const method.
std::vector<int>* inst_count = nullptr; // for profiling std::vector<int>* inst_count = nullptr; // for profiling
double* CPU_time = nullptr; // cumulative CPU time for the program double* CPU_time = nullptr; // cumulative CPU time for the program
std::vector<double>* inst_CPU; // per-instruction CPU time. std::vector<double>* inst_CPU = nullptr; // per-instruction CPU time.
CaseMaps<bro_int_t> int_cases; CaseMaps<bro_int_t> int_cases;
CaseMaps<bro_uint_t> uint_cases; CaseMaps<bro_uint_t> uint_cases;