diff --git a/CHANGES b/CHANGES index 4a56ec3dbb..6008de7624 100644 --- a/CHANGES +++ b/CHANGES @@ -1,4 +1,38 @@ +3.2.0-dev.137 | 2020-02-24 20:41:43 -0800 + + * parse.y: fix memory leak in FieldAssignExpr call (Max Kellermann) + + * parse.y: fix use-after-free bug in open-ended index_slice (Max Kellermann) + + * Type: fix use-after-free bug in init_type() (Max Kellermann) + + * Expr: fix potential memory leak in RecordCoerceExpr::Fold() (Max Kellermann) + + * Expr: fix memory leak in RecordCoerceExpr::InitVal() (Max Kellermann) + + * zeekygen/IdentifierInfo: fix memory leak in operator=() (Max Kellermann) + + * Func: fix memory leaks in get_func_priority() (Max Kellermann) + + * parse.y: fix several memory leaks after lookup_ID() (Max Kellermann) + + * Func: fix memory leaks in check_built_in_call() (Max Kellermann) + + * Var: fix memory leaks in add_global() and add_local() (Max Kellermann) + + * Var: add missing references to `init` in add{,_and_assign}_local() (Max Kellermann) + + * parse.y: hold reference on init_expr for zeekygen::Manager::Redef() (Max Kellermann) + + * Expr: fix two memory leaks in AssignExpr::InitVal() (Max Kellermann) + + * parse.y: fix memory leak after "&deprecated" without string (Max Kellermann) + + * RuleMatcher: delete PatternSet instances in destructor (Max Kellermann) + + * Fix reference counting in Option::set_change_handler() (Max Kellermann) + 3.2.0-dev.120 | 2020-02-24 18:13:04 -0800 * Update zeek-testing commit (Jon Siwek, Corelight) diff --git a/VERSION b/VERSION index 373f2e48d3..6daa8b5721 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -3.2.0-dev.120 +3.2.0-dev.137 diff --git a/src/Expr.cc b/src/Expr.cc index 80b4e18685..fb2583fc6c 100644 --- a/src/Expr.cc +++ b/src/Expr.cc @@ -2479,6 +2479,7 @@ Val* AssignExpr::InitVal(const BroType* t, Val* aggr) const const BroType* yt = tv->Type()->YieldType(); IntrusivePtr index{AdoptRef{}, op1->InitVal(tt->Indices(), 0)}; IntrusivePtr v{AdoptRef{}, op2->InitVal(yt, 0)}; + if ( ! index || ! v ) return 0; @@ -3770,12 +3771,10 @@ Val* RecordCoerceExpr::InitVal(const BroType* t, Val* aggr) const { RecordVal* rv = v->AsRecordVal(); RecordVal* ar = rv->CoerceTo(t->AsRecordType(), aggr); + Unref(rv); if ( ar ) - { - Unref(rv); return ar; - } } Error("bad record initializer"); @@ -3800,8 +3799,7 @@ Val* RecordCoerceExpr::Fold(Val* v) const if ( def ) rhs = def->AttrExpr()->Eval(0); } - - if ( rhs ) + else rhs = rhs->Ref(); assert(rhs || Type()->AsRecordType()->FieldDecl(i)->FindAttr(ATTR_OPTIONAL)); diff --git a/src/Func.cc b/src/Func.cc index cd5062af9b..639d69f685 100644 --- a/src/Func.cc +++ b/src/Func.cc @@ -828,6 +828,7 @@ bool check_built_in_call(BuiltinFunc* f, CallExpr* call) if ( ! *fmt_str ) { + Unref(fmt_str_val); call->Error("format string ends with bare '%'"); return false; } @@ -839,11 +840,13 @@ bool check_built_in_call(BuiltinFunc* f, CallExpr* call) if ( args.length() != num_fmt + 1 ) { + Unref(fmt_str_val); call->Error("mismatch between format string to fmt() and number of arguments passed"); return false; } } + Unref(fmt_str_val); return true; } @@ -873,11 +876,13 @@ static int get_func_priority(const attr_list& attrs) if ( ! IsIntegral(v->Type()->Tag()) ) { + Unref(v); a->Error("expression is not of integral type"); continue; } priority = v->InternalInt(); + Unref(v); } return priority; diff --git a/src/RuleMatcher.cc b/src/RuleMatcher.cc index 520d34109f..cc3e59448a 100644 --- a/src/RuleMatcher.cc +++ b/src/RuleMatcher.cc @@ -130,7 +130,10 @@ RuleHdrTest::~RuleHdrTest() for ( int i = 0; i < Rule::TYPES; ++i ) { for ( auto pset : psets[i] ) + { delete pset->re; + delete pset; + } } delete ruleset; diff --git a/src/Type.cc b/src/Type.cc index 88fffa4724..d673120158 100644 --- a/src/Type.cc +++ b/src/Type.cc @@ -2100,7 +2100,20 @@ BroType* init_type(Expr* init) BroType* t = e0->InitType(); if ( t ) + { + BroType* old_t = t; t = reduce_type(t); + + if ( t ) + // reduce_type() does not return a referenced pointer, but we want + // to own a reference, so create one here + Ref(t); + + // reduce_type() does not adopt our reference passed as parameter, so + // we need to release it (after the Ref() call above) + Unref(old_t); + } + if ( ! t ) return 0; diff --git a/src/Var.cc b/src/Var.cc index 971a09cfa9..c1d9b61022 100644 --- a/src/Var.cc +++ b/src/Var.cc @@ -42,6 +42,7 @@ static void make_var(ID* id, BroType* t, init_class c, Expr* init, else if ( dt != VAR_REDEF || init || ! attr ) { id->Error("already defined", init); + Unref(init); return; } } @@ -51,6 +52,7 @@ static void make_var(ID* id, BroType* t, init_class c, Expr* init, if ( ! id->Type() ) { id->Error("\"redef\" used but not previously defined"); + Unref(init); return; } @@ -64,6 +66,7 @@ static void make_var(ID* id, BroType* t, init_class c, Expr* init, (! init || ! do_init || (! t && ! (t = init_type(init)))) ) { id->Error("already defined", init); + Unref(init); return; } @@ -71,6 +74,7 @@ static void make_var(ID* id, BroType* t, init_class c, Expr* init, if ( ! same_type(t, id->Type()) ) { id->Error("redefinition changes type", init); + Unref(init); return; } } @@ -85,6 +89,7 @@ static void make_var(ID* id, BroType* t, init_class c, Expr* init, if ( init ) { id->Error("double initialization", init); + Unref(init); return; } @@ -98,6 +103,7 @@ static void make_var(ID* id, BroType* t, init_class c, Expr* init, if ( ! init ) { id->Error("no type given"); + Unref(init); return; } @@ -105,6 +111,7 @@ static void make_var(ID* id, BroType* t, init_class c, Expr* init, if ( ! t ) { id->SetType(error_type()); + Unref(init); return; } } @@ -185,7 +192,10 @@ static void make_var(ID* id, BroType* t, init_class c, Expr* init, { v = init_val(init, t, aggr); if ( ! v ) + { + Unref(init); return; + } } if ( aggr ) @@ -211,6 +221,8 @@ static void make_var(ID* id, BroType* t, init_class c, Expr* init, id->SetOption(); } + Unref(init); + id->UpdateValAttrs(); if ( t && t->Tag() == TYPE_FUNC && @@ -235,7 +247,7 @@ void add_global(ID* id, BroType* t, init_class c, Expr* init, Stmt* add_local(ID* id, BroType* t, init_class c, Expr* init, attr_list* attr, decl_type dt) { - make_var(id, t, c, init, attr, dt, 0); + make_var(id, t, c, init ? init->Ref() : init, attr, dt, 0); if ( init ) { @@ -262,7 +274,7 @@ Stmt* add_local(ID* id, BroType* t, init_class c, Expr* init, extern Expr* add_and_assign_local(ID* id, Expr* init, Val* val) { - make_var(id, 0, INIT_FULL, init, 0, VAR_REGULAR, 0); + make_var(id, 0, INIT_FULL, init->Ref(), 0, VAR_REGULAR, 0); Ref(id); return new AssignExpr(new NameExpr(id), init, 0, val); } diff --git a/src/option.bif b/src/option.bif index 55e3c4a6cf..8c56fe4c38 100644 --- a/src/option.bif +++ b/src/option.bif @@ -205,6 +205,8 @@ function Option::set_change_handler%(ID: string, on_change: any, priority: int & return val_mgr->GetBool(0); } - i->AddOptionHandler(on_change->Ref()->AsFunc(), -priority); + auto* func = on_change->AsFunc(); + Ref(func); + i->AddOptionHandler(func, -priority); return val_mgr->GetBool(1); %} diff --git a/src/parse.y b/src/parse.y index 6106b04482..9033c08635 100644 --- a/src/parse.y +++ b/src/parse.y @@ -92,6 +92,7 @@ #include "Brofiler.h" #include "zeekygen/Manager.h" #include "module_util.h" +#include "IntrusivePtr.h" #include #include @@ -514,7 +515,8 @@ expr: } func_body { - $$ = new FieldAssignExpr($2, new ConstExpr(func_id->ID_Val())); + $$ = new FieldAssignExpr($2, new ConstExpr(func_id->ID_Val()->Ref())); + Unref(func_id); } | expr TOK_IN expr @@ -688,6 +690,7 @@ expr: { id->Error("undeclared variable"); id->SetType(error_type()); + Ref(id); $$ = new NameExpr(id); } @@ -701,10 +704,15 @@ expr: $$ = new ConstExpr(t->GetVal(intval)); } else + { + Ref(id); $$ = new NameExpr(id); + } if ( id->IsDeprecated() ) reporter->Warning("%s", id->GetDeprecationWarning().c_str()); + + Unref(id); } } @@ -1099,6 +1107,7 @@ decl: | TOK_REDEF global_id opt_type init_class opt_init opt_attr ';' { + IntrusivePtr e{NewRef{}, $5}; add_global($2, $3, $4, $5, $6, VAR_REDEF); zeekygen_mgr->Redef($2, ::filename, $4, $5); } @@ -1296,7 +1305,7 @@ index_slice: { set_location(@1, @6); Expr* low = $3 ? $3 : new ConstExpr(val_mgr->GetCount(0)); - Expr* high = $5 ? $5 : new SizeExpr($1); + Expr* high = $5 ? $5 : new SizeExpr($1->Ref()); if ( ! IsIntegral(low->Type()->Tag()) || ! IsIntegral(high->Type()->Tag()) ) reporter->Error("slice notation must have integral values as indexes"); @@ -1363,6 +1372,7 @@ attr: { ODesc d; $3->Describe(&d); + Unref($3); reporter->Error("'&deprecated=%s' must use a string literal", d.Description()); $$ = new Attr(ATTR_DEPRECATED); @@ -1577,6 +1587,8 @@ event: } if ( id->IsDeprecated() ) reporter->Warning("%s", id->GetDeprecationWarning().c_str()); + + Unref(id); } $$ = new EventExpr($1, $3); @@ -1628,7 +1640,10 @@ case_type: if ( case_var && case_var->IsGlobal() ) case_var->Error("already a global identifier"); else + { + Unref(case_var); case_var = install_ID(name, current_module.c_str(), false, false); + } add_local(case_var, type, INIT_NONE, 0, 0, VAR_REGULAR); $$ = case_var; @@ -1652,8 +1667,11 @@ for_head: } else + { + Unref(loop_var); loop_var = install_ID($3, current_module.c_str(), false, false); + } id_list* loop_vars = new id_list; loop_vars->push_back(loop_var); diff --git a/src/zeekygen/IdentifierInfo.cc b/src/zeekygen/IdentifierInfo.cc index 5ff5b5dfa3..36f5036f73 100644 --- a/src/zeekygen/IdentifierInfo.cc +++ b/src/zeekygen/IdentifierInfo.cc @@ -165,6 +165,8 @@ IdentifierInfo::Redefinition::operator=(const IdentifierInfo::Redefinition& othe if ( &other == this ) return *this; + Unref(init_expr); + from_script = other.from_script; ic = other.ic; init_expr = other.init_expr;