BIT-1909: fix invalid redef'd record field accesses

src/Net.cc

@@ -61,6 +61,7 @@ double bro_start_time = 0.0; // time Bro started.
 double bro_start_network_time;	// timestamp of first packet
 double last_watchdog_proc_time = 0.0;	// value of above during last watchdog
 bool terminating = false;	// whether we're done reading and finishing up
+bool is_parsing = false;
 
 const Packet *current_pkt = 0;
 int current_dispatched = 0;

src/Net.h

@@ -70,6 +70,9 @@ extern bool terminating;
 // True if the remote serializer is to be activated.
 extern bool using_communication;
 
+// True if Bro is currently parsing scripts.
+extern bool is_parsing;
+
 extern const Packet* current_pkt;
 extern int current_dispatched;
 extern double current_timestamp;

src/Val.cc

@@ -2722,6 +2722,8 @@ unsigned int TableVal::MemoryAllocation() const
 		+ table_hash->MemoryAllocation();
 	}
 
+vector<RecordVal*> RecordVal::parse_time_records;
+
 RecordVal::RecordVal(RecordType* t) : MutableVal(t)
 	{
 	origin = 0;
@@ -2767,6 +2769,12 @@ RecordVal::RecordVal(RecordType* t) : MutableVal(t)
 		vl->append(def ? def->Ref() : 0);
 
 		Unref(def);
+
+		if ( is_parsing )
+			{
+			parse_time_records.emplace_back(this);
+			Ref();
+			}
 		}
 	}
 
@@ -2832,6 +2840,24 @@ Val* RecordVal::LookupWithDefault(int field) const
 	return record_type->FieldDefault(field);
 	}
 
+bool RecordVal::Resize()
+	{
+	auto vs = val.val_list_val;
+	auto rt = record_type;
+	auto current_length = vs->length();
+	auto required_length = rt->NumFields();
+
+	if ( required_length <= current_length )
+		return false;
+
+	vs->resize(required_length);
+
+	for ( auto i = current_length; i < required_length; ++i )
+		vs->replace(i, nullptr);
+
+	return true;
+	}
+
 Val* RecordVal::Lookup(const char* field, bool with_default) const
 	{
 	int idx = record_type->FieldOffset(field);

src/Val.h

@@ -920,6 +920,8 @@ protected:
 
 class RecordVal : public MutableVal {
 public:
+	static vector<RecordVal*> parse_time_records;
+
 	explicit RecordVal(RecordType* t);
 	~RecordVal() override;
 
@@ -930,6 +932,11 @@ public:
 	Val* Lookup(int field) const;	// Does not Ref() value.
 	Val* LookupWithDefault(int field) const;	// Does Ref() value.
 
+	// Extend the underlying array to match the number of fields in the
+	// record type (they may mismatch as a result of parse-time record type
+	// redefinitions.
+	bool Resize();
+
 	/**
 	 * 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.

src/main.cc

@@ -854,7 +854,17 @@ int main(int argc, char** argv)
 	HeapLeakChecker::Disabler disabler;
 #endif
 
+	is_parsing = true;
 	yyparse();
+	is_parsing = false;
+
+	for ( auto& rv : RecordVal::parse_time_records )
+		{
+		rv->Resize();
+		Unref(rv);
+		}
+
+	RecordVal::parse_time_records = {};
 
 	init_general_global_var();
 	init_net_var();

testing/btest/Baseline/language.record-redef-after-init/output

@@ -0,0 +1,10 @@
+[a=redef, d=<uninitialized>, e=<uninitialized>, f=<uninitialized>, g=<uninitialized>, h=<uninitialized>, i=<uninitialized>, j=<uninitialized>, k=<uninitialized>, l=<uninitialized>, m=<uninitialized>, n=<uninitialized>, o=<uninitialized>, p=<uninitialized>, q=<uninitialized>]
+[a=runtime, d=<uninitialized>, e=<uninitialized>, f=<uninitialized>, g=<uninitialized>, h=<uninitialized>, i=<uninitialized>, j=<uninitialized>, k=<uninitialized>, l=<uninitialized>, m=<uninitialized>, n=<uninitialized>, o=<uninitialized>, p=<uninitialized>, q=OPTQ]
+[a=local, d=<uninitialized>, e=<uninitialized>, f=<uninitialized>, g=<uninitialized>, h=<uninitialized>, i=<uninitialized>, j=<uninitialized>, k=<uninitialized>, l=<uninitialized>, m=<uninitialized>, n=<uninitialized>, o=<uninitialized>, p=<uninitialized>, q=OPTQ]
+[a=redef, d=<uninitialized>, e=<uninitialized>, f=<uninitialized>, g=<uninitialized>, h=<uninitialized>, i=<uninitialized>, j=<uninitialized>, k=<uninitialized>, l=<uninitialized>, m=<uninitialized>, n=<uninitialized>, o=<uninitialized>, p=<uninitialized>, q=<uninitialized>]
+[a=redef, d=<uninitialized>, e=<uninitialized>, f=<uninitialized>, g=<uninitialized>, h=<uninitialized>, i=<uninitialized>, j=<uninitialized>, k=<uninitialized>, l=<uninitialized>, m=<uninitialized>, n=<uninitialized>, o=<uninitialized>, p=<uninitialized>, q=<uninitialized>]
+newp
+[a=redef, d=<uninitialized>, e=<uninitialized>, f=<uninitialized>, g=<uninitialized>, h=<uninitialized>, i=<uninitialized>, j=<uninitialized>, k=<uninitialized>, l=<uninitialized>, m=<uninitialized>, n=<uninitialized>, o=<uninitialized>, p=newp, q=<uninitialized>]
+OPTQ
+our value
+[a=redef, d=<uninitialized>, e=<uninitialized>, f=<uninitialized>, g=<uninitialized>, h=<uninitialized>, i=<uninitialized>, j=<uninitialized>, k=<uninitialized>, l=<uninitialized>, m=<uninitialized>, n=<uninitialized>, o=<uninitialized>, p=newp, q=our value]

testing/btest/language/record-redef-after-init.bro

@@ -0,0 +1,52 @@
+# @TEST-EXEC: bro -b %INPUT >output
+# @TEST-EXEC: btest-diff output
+
+type myrec: record {
+    a: string;
+};
+
+const mr = myrec($a = "init") &redef;
+
+redef mr = myrec($a = "redef");
+
+# Many fields may help ensure out-of-bounds reference failures
+redef record myrec += {
+    d: string &optional;
+    e: string &optional;
+    f: string &optional;
+    g: string &optional;
+    h: string &optional;
+    i: string &optional;
+    j: string &optional;
+    k: string &optional;
+    l: string &optional;
+    m: string &optional;
+    n: string &optional;
+    o: string &optional;
+    p: string &optional;
+    q: string &default="OPTQ";
+};
+
+print mr;                        # original 'myrec' type with updated a value
+print myrec($a = "runtime");     # check we get new defaults
+
+local mr2 = myrec($a = "local");
+print mr2;
+
+mr2 = mr;      # Copying should do the right thing
+print mr2;
+
+local mr3: myrec = mr; # Initializing should do the right thing
+print mr3;
+
+if ( mr?$q )    # the test that did not work properly
+    {
+    print mr$q; # accessed invalid memory location
+    }
+mr$p = "newp";  # Assignment updates mr as much as needed
+print mr$p;
+print mr;
+print mr$q;
+mr$q = "our value";
+print mr$q;
+print mr;