Fix segfault when an existing enum identifier is added again with a

different value.

Addresses BIT-931.

Also switching the internal enum ID map to storing std::string for
easier memory management.

CHANGES

@@ -1,4 +1,12 @@
 
+2.4-679 | 2016-07-05 16:35:53 -0700
+
+  * Fix segfault when an existing enum identifier is added again with
+    a different value. Addresses BIT-931. (Robin Sommer)
+
+  * Escape the empty indicator in logs if it occurs literally as a
+    field's actual content. Addresses BIT-931. (Robin Sommer)
+
 2.4-676 | 2016-06-30 17:27:54 -0700
 
   * A larger series of NetControl updates. (Johanna Amann)
@@ -9,7 +17,7 @@
           this action did nothing by default.
 
         * Rewrite of catch-and-release.
-          
+
         * Fix several small logging issues.
 
         * find_rules_subnet() now works in cluster mode. This
@@ -19,15 +27,15 @@
           tracking.
 
         * Fix acld whitelist command.
-          
+
         * Add rule existance as a state besides added and failure.
-      
+
         * Suppress duplicate "plugin activated" messages.
-      
+
         * Make new Broker plugin options accessible.
-          
+
         * Add predicates to Broker plugin.
-      
+
   * Tweak SMTP scripts to not to pull in the notice framework.
 
 2.4-658 | 2016-06-30 16:55:32 -0700
@@ -44,7 +52,7 @@
 
 2.4-644 | 2016-06-21 13:59:05 -0400
 
-  * Fix an off-by-one error when grabbing x-originating-ip header in 
+  * Fix an off-by-one error when grabbing x-originating-ip header in
     email. (Seth Hall, Aashish Sharma)
 
 2.4-642 | 2016-06-18 13:18:23 -0700

VERSION

@@ -1 +1 @@
-2.4-676
+2.4-679

src/Type.cc

@@ -1437,13 +1437,11 @@ EnumType::EnumType(EnumType* e)
 	SetName(e->GetName());
 
 	for ( NameMap::iterator it = e->names.begin(); it != e->names.end(); ++it )
-		names[copy_string(it->first)] = it->second;
+		names[it->first] = it->second;
 	}
 
 EnumType::~EnumType()
 	{
-	for ( NameMap::iterator iter = names.begin(); iter != names.end(); ++iter )
-		delete [] iter->first;
 	}
 
 // Note, we use reporter->Error() here (not Error()) to include the current script
@@ -1503,8 +1501,10 @@ void EnumType::CheckAndAddName(const string& module_name, const char* name,
 		// We allow double-definitions if matching exactly. This is so that
 		// we can define an enum both in a *.bif and *.bro for avoiding
 		// cyclic dependencies.
-		if ( id->Name() != make_full_var_name(module_name.c_str(), name)
+		string fullname = make_full_var_name(module_name.c_str(), name);
-		     || (id->HasVal() && val != id->ID_Val()->AsEnum()) )
+		if ( id->Name() != fullname
+		     || (id->HasVal() && val != id->ID_Val()->AsEnum())
+		     || (names.find(fullname) != names.end() && names[fullname] != val) )
 			{
 			Unref(id);
 			reporter->Error("identifier or enumerator value in enumerated type definition already exists");
@@ -1530,7 +1530,7 @@ void EnumType::AddNameInternal(const string& module_name, const char* name,
                                bro_int_t val, bool is_export)
 	{
 	string fullname = make_full_var_name(module_name.c_str(), name);
-	names[copy_string(fullname.c_str())] = val;
+	names[fullname] = val;
 	}
 
 bro_int_t EnumType::Lookup(const string& module_name, const char* name) const
@@ -1549,7 +1549,7 @@ const char* EnumType::Lookup(bro_int_t value) const
 	for ( NameMap::const_iterator iter = names.begin();
 	      iter != names.end(); ++iter )
 		if ( iter->second == value )
-			return iter->first;
+			return iter->first.c_str();
 
 	return 0;
 	}
@@ -1570,7 +1570,7 @@ void EnumType::DescribeReST(ODesc* d, bool roles_only) const
 
 	// Create temporary, reverse name map so that enums can be documented
 	// in ascending order of their actual integral value instead of by name.
-	typedef map< bro_int_t, const char* > RevNameMap;
+	typedef map<bro_int_t, std::string> RevNameMap;
 
 	RevNameMap rev;
 
@@ -1583,9 +1583,9 @@ void EnumType::DescribeReST(ODesc* d, bool roles_only) const
 		d->PushIndent();
 
 		if ( roles_only )
-			d->Add(fmt(":bro:enum:`%s`", it->second));
+			d->Add(fmt(":bro:enum:`%s`", it->second.c_str()));
 		else
-			d->Add(fmt(".. bro:enum:: %s %s", it->second, GetName().c_str()));
+			d->Add(fmt(".. bro:enum:: %s %s", it->second.c_str(), GetName().c_str()));
 
 		using broxygen::IdentifierInfo;
 		IdentifierInfo* doc = broxygen_mgr->GetIdentifierInfo(it->second);
@@ -1593,7 +1593,7 @@ void EnumType::DescribeReST(ODesc* d, bool roles_only) const
 		if ( ! doc )
 			{
 			reporter->InternalWarning("Enum %s documentation lookup failure",
-			                          it->second);
+			                          it->second.c_str());
 			continue;
 			}
 

src/Type.h

@@ -584,7 +584,7 @@ protected:
 	                     const char* name, bro_int_t val, bool is_export,
 	                     bool deprecated);
 
-	typedef std::map< const char*, bro_int_t, ltstr > NameMap;
+	typedef std::map<std::string, bro_int_t> NameMap;
 	NameMap names;
 
 	// The counter is initialized to 0 and incremented on every implicit

testing/btest/core/enum-redef-exists.bro

@@ -0,0 +1,7 @@
+# @TEST-EXEC: bro -b %INPUT >output
+
+module SSH;
+
+export {
+	redef enum Log::ID += { LOG };
+}