Merge remote-tracking branch 'origin/topic/jsiwek/gh-541-ntlm-fix'

* origin/topic/jsiwek/gh-541-ntlm-fix:
  GH-541: add test cases for NTLM AV Pair sequence handling
  GH-541: fix handling of NTLM AV Pair sequences

src/analyzer/protocol/ntlm/ntlm-analyzer.pac

@@ -24,13 +24,26 @@ refine connection NTLM_Conn += {
 		return result;
 		%}
 
-	function build_av_record(val: NTLM_AV_Pair_Sequence): BroVal
+	function build_av_record(val: NTLM_AV_Pair_Sequence, len: uint16): BroVal
 		%{
 		RecordVal* result = new RecordVal(BifType::Record::NTLM::AVs);
-		for ( uint i = 0; ${val.pairs[i].id} != 0; i++ )
+		for ( uint i = 0; ; i++ )
 			{
+			if ( i >= ${val.pairs}->size() )
+				{
+				if ( len != 0 )
+					// According to spec, the TargetInfo MUST be a sequence of
+					// AV_PAIRs and terminated by the null AV_PAIR when the
+					// TargetInfoLen is non-zero, so this is in violation.
+					bro_analyzer()->ProtocolViolation("NTLM AV Pair loop underflow");
+
+				return result;
+				}
+
 			switch ( ${val.pairs[i].id} )
 				{
+				case 0:
+					return result;
 				case 1:
 					result->Assign(0, utf16_bytestring_to_utf8_val(bro_analyzer()->Conn(), ${val.pairs[i].nb_computer_name.data}));
 					break;
@@ -131,7 +144,7 @@ refine connection NTLM_Conn += {
 			result->Assign(2, build_version_record(${val.version}));
 
 		if ( ${val}->has_target_info() )
-			result->Assign(3, build_av_record(${val.target_info}));
+			result->Assign(3, build_av_record(${val.target_info},  ${val.target_info_fields.length}));
 
 		BifEvent::generate_ntlm_challenge(bro_analyzer(),
 		                                  bro_analyzer()->Conn(),

testing/btest/Baseline/scripts.base.protocols.dce-rpc.ntlm-empty-av-pair-seq/dpd.log

@@ -0,0 +1,10 @@
+#separator \x09
+#set_separator	,
+#empty_field	(empty)
+#unset_field	-
+#path	dpd
+#open	2019-08-26-17-26-38
+#fields	ts	uid	id.orig_h	id.orig_p	id.resp_h	id.resp_p	proto	analyzer	failure_reason
+#types	time	string	addr	port	addr	port	enum	string	string
+1056991898.901892	CHhAvVGS1DHFjwGM9	192.168.0.173	1068	192.168.0.2	4997	tcp	NTLM	NTLM AV Pair loop underflow
+#close	2019-08-26-17-26-38

testing/btest/Baseline/scripts.base.protocols.dce-rpc.ntlm-empty-av-pair-seq/ntlm.log

@@ -0,0 +1,10 @@
+#separator \x09
+#set_separator	,
+#empty_field	(empty)
+#unset_field	-
+#path	ntlm
+#open	2019-08-26-17-26-38
+#fields	ts	uid	id.orig_h	id.orig_p	id.resp_h	id.resp_p	username	hostname	domainname	server_nb_computer_name	server_dns_computer_name	server_tree_name	success
+#types	time	string	addr	port	addr	port	string	string	string	string	string	string	bool
+1056991898.900518	CHhAvVGS1DHFjwGM9	192.168.0.173	1068	192.168.0.2	4997	-	-	-	-	-	-	-
+#close	2019-08-26-17-26-38

testing/btest/Baseline/scripts.base.protocols.dce-rpc.ntlm-unterminated-av-pair-seq/dpd.log

@@ -0,0 +1,10 @@
+#separator \x09
+#set_separator	,
+#empty_field	(empty)
+#unset_field	-
+#path	dpd
+#open	2019-08-26-17-26-39
+#fields	ts	uid	id.orig_h	id.orig_p	id.resp_h	id.resp_p	proto	analyzer	failure_reason
+#types	time	string	addr	port	addr	port	enum	string	string
+1056991898.901892	CHhAvVGS1DHFjwGM9	192.168.0.173	1068	192.168.0.2	4997	tcp	NTLM	NTLM AV Pair loop underflow
+#close	2019-08-26-17-26-39

testing/btest/Baseline/scripts.base.protocols.dce-rpc.ntlm-unterminated-av-pair-seq/ntlm.log

@@ -0,0 +1,10 @@
+#separator \x09
+#set_separator	,
+#empty_field	(empty)
+#unset_field	-
+#path	ntlm
+#open	2019-08-26-17-26-39
+#fields	ts	uid	id.orig_h	id.orig_p	id.resp_h	id.resp_p	username	hostname	domainname	server_nb_computer_name	server_dns_computer_name	server_tree_name	success
+#types	time	string	addr	port	addr	port	string	string	string	string	string	string	bool
+1056991898.900518	CHhAvVGS1DHFjwGM9	192.168.0.173	1068	192.168.0.2	4997	-	-	-	SATURN	-	-	-
+#close	2019-08-26-17-26-39

testing/btest/scripts/base/protocols/dce-rpc/ntlm-empty-av-pair-seq.zeek

@@ -0,0 +1,8 @@
+# Tests for good parsing/handling of empty NTLM AV Pair sequences.
+
+# @TEST-EXEC: zeek -b -r $TRACES/dce-rpc/ntlm-empty-av-sequence.pcap %INPUT
+# @TEST-EXEC: btest-diff ntlm.log
+# @TEST-EXEC: btest-diff dpd.log
+
+@load base/protocols/dce-rpc
+@load base/protocols/ntlm

testing/btest/scripts/base/protocols/dce-rpc/ntlm-unterminated-av-pair-seq.zeek

@@ -0,0 +1,8 @@
+# Tests for good parsing/handling of unterminated NTLM AV Pair sequences.
+
+# @TEST-EXEC: zeek -b -r $TRACES/dce-rpc/ntlm-unterminated-av-sequence.pcap %INPUT
+# @TEST-EXEC: btest-diff ntlm.log
+# @TEST-EXEC: btest-diff dpd.log
+
+@load base/protocols/dce-rpc
+@load base/protocols/ntlm