Fix possible buffer over-read in DNS TSIG parsing

src/analyzer/protocol/dns/DNS.cc

@@ -701,6 +701,19 @@ int DNS_Interpreter::ParseRR_EDNS(DNS_MsgInfo* msg,
 	return 1;
 	}
 
+void DNS_Interpreter::ExtractOctets(const u_char*& data, int& len,
+                                    BroString** p)
+	{
+	uint16 dlen = ExtractShort(data, len);
+	dlen = min(len, static_cast<int>(dlen));
+
+	if ( p )
+		*p = new BroString(data, dlen, 0);
+
+	data += dlen;
+	len -= dlen;
+	}
+
 int DNS_Interpreter::ParseRR_TSIG(DNS_MsgInfo* msg,
 				const u_char*& data, int& len, int rdlength,
 				const u_char* msg_start)
@@ -718,24 +731,17 @@ int DNS_Interpreter::ParseRR_TSIG(DNS_MsgInfo* msg,
 	uint32 sign_time_sec = ExtractLong(data, len);
 	unsigned int sign_time_msec = ExtractShort(data, len);
 	unsigned int fudge = ExtractShort(data, len);
-
+	BroString* request_MAC;
-	u_char request_MAC[16];
+	ExtractOctets(data, len, &request_MAC);
-	memcpy(request_MAC, data, sizeof(request_MAC));
-
-	// Here we adjust the size of the requested MAC + u_int16_t
-	// for length.  See RFC 2845, sec 2.3.
-	int n = sizeof(request_MAC) + sizeof(u_int16_t);
-	data += n;
-	len -= n;
-
 	unsigned int orig_id = ExtractShort(data, len);
 	unsigned int rr_error = ExtractShort(data, len);
+	ExtractOctets(data, len, 0);  // Other Data
 
 	msg->tsig = new TSIG_DATA;
 
 	msg->tsig->alg_name =
 		new BroString(alg_name, alg_name_end - alg_name, 1);
-	msg->tsig->sig = new BroString(request_MAC, sizeof(request_MAC), 1);
+	msg->tsig->sig = request_MAC;
 	msg->tsig->time_s = sign_time_sec;
 	msg->tsig->time_ms = sign_time_msec;
 	msg->tsig->fudge = fudge;

src/analyzer/protocol/dns/DNS.h

@@ -180,6 +180,7 @@ protected:
 
 	uint16 ExtractShort(const u_char*& data, int& len);
 	uint32 ExtractLong(const u_char*& data, int& len);
+	void ExtractOctets(const u_char*& data, int& len, BroString** p);
 
 	int ParseRR_Name(DNS_MsgInfo* msg,
 				const u_char*& data, int& len, int rdlength,

testing/btest/Baseline/scripts.base.protocols.dns.tsig/out

@@ -0,0 +1,2 @@
+[query=secret-key, qtype=3, alg_name=hmac-md5.sig-alg.reg.int, sig=F\xbd\xbf1\xef^B6\xb8\xeb\xae1u,\x87\xdb^?, time_signed=21513.794, fudge=300.0, orig_id=9703, rr_error=0, is_query=1]
+16

testing/btest/scripts/base/protocols/dns/tsig.bro

@@ -0,0 +1,10 @@
+# @TEST-EXEC: bro -r $TRACES/dns-tsig.trace %INPUT >out
+# @TEST-EXEC: btest-diff out
+
+redef dns_skip_all_addl = F;
+
+event dns_TSIG_addl(c: connection, msg: dns_msg, ans: dns_tsig_additional)
+	{
+	print ans;
+	print |ans$sig|;
+	}