Merge branch 'leaks' of https://github.com/MaxKellermann/zeek

- Various minor code formatting/styling during the merge

* 'leaks' of https://github.com/MaxKellermann/zeek:
  parse.y: fix memory leak in FieldAssignExpr call
  parse.y: fix use-after-free bug in open-ended index_slice
  Type: fix use-after-free bug in init_type()
  Expr: fix memory leak in RecordCoerceExpr::Fold()
  Expr: fix memory leak in RecordCoerceExpr::InitVal()
  zeekygen/IdentifierInfo: fix memory leak in operator=()
  Func: fix memory leaks in get_func_priority()
  parse.y: fix several memory leaks after lookup_ID()
  Func: fix memory leaks in check_built_in_call()
  Var: fix memory leaks in add_global() and add_local()
  Var: add missing references to `init` in add{,_and_assign}_local()
  parse.y: hold reference on init_expr for zeekygen::Manager::Redef()
  Expr: fix two memory leaks in AssignExpr::InitVal()
  parse.y: fix memory leak after "&derepcated" without string
  RuleMatcher: delete PatternSet instances in destructor (memleak)
  option.bif: fix crash bug by referencing `Func`, not `Val`

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)

VERSION

@@ -1 +1 @@
-3.2.0-dev.120
+3.2.0-dev.137

src/Expr.cc

@@ -2479,6 +2479,7 @@ Val* AssignExpr::InitVal(const BroType* t, Val* aggr) const
 		const BroType* yt = tv->Type()->YieldType();
 		IntrusivePtr<Val> index{AdoptRef{}, op1->InitVal(tt->Indices(), 0)};
 		IntrusivePtr<Val> v{AdoptRef{}, op2->InitVal(yt, 0)};
+
 		if ( ! index || ! v )
 			return 0;
 
@@ -3770,13 +3771,11 @@ 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");
 	return 0;
@@ -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));

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;

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;

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;
 

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,8 +192,11 @@ 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 )
 				id->SetVal(aggr, c);
@@ -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);
 	}

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);
 	%}

src/parse.y

@@ -92,6 +92,7 @@
 #include "Brofiler.h"
 #include "zeekygen/Manager.h"
 #include "module_util.h"
+#include "IntrusivePtr.h"
 
 #include <set>
 #include <string>
@@ -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<Expr> 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);

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;