CVE-2017-12994/BGP: Move a test inside a loop.

The loop can be executed more than once (that's kinda the whole point of
a loop), so the check has to be made each time through the loop, not
just once before the loop is executed.

Do some additional length checks while we're at it.

This fixes a buffer over-read discovered by Forcepoint's security
researchers Otto Airamo & Antti Levomäki.

Add a test using the capture file supplied by the reporter(s).

print-bgp.c

@@ -2182,35 +2182,42 @@ bgp_attr_print(netdissect_options *ndo,
 		uint8_t type;
 		uint16_t length;
 
-		ND_TCHECK2(tptr[0], 3);
-
 		tlen = len;
 
 		while (tlen >= 3) {
 
+		    ND_TCHECK2(tptr[0], 3);
+
 		    type = *tptr;
 		    length = EXTRACT_16BITS(tptr+1);
+		    tptr += 3;
+		    tlen -= 3;
 
 		    ND_PRINT((ndo, "\n\t    %s TLV (%u), length %u",
 			      tok2str(bgp_aigp_values, "Unknown", type),
 			      type, length));
 
+		    if (length < 3)
+			goto trunc;
+		    length -= 3;
+
 		    /*
 		     * Check if we can read the TLV data.
 		     */
-		    ND_TCHECK2(tptr[3], length - 3);
+		    ND_TCHECK2(tptr[3], length);
 
 		    switch (type) {
 
 		    case BGP_AIGP_TLV:
-		        ND_TCHECK2(tptr[3], 8);
+		        if (length < 8)
+		            goto trunc;
 			ND_PRINT((ndo, ", metric %" PRIu64,
-				  EXTRACT_64BITS(tptr+3)));
+				  EXTRACT_64BITS(tptr)));
 			break;
 
 		    default:
 			if (ndo->ndo_vflag <= 1) {
-			    print_unknown_data(ndo, tptr+3,"\n\t      ", length-3);
+			    print_unknown_data(ndo, tptr,"\n\t      ", length);
 			}
 		    }
 

tests/TESTLIST

@@ -473,6 +473,7 @@ zephyr-oobr		zephyr-oobr.pcap		zephyr-oobr.out		-vvv -e
 isakmp-no-none-np	isakmp-no-none-np.pcap		isakmp-no-none-np.out	-vvv -e
 telnet-iac-check-oobr	telnet-iac-check-oobr.pcap	telnet-iac-check-oobr.out	-vvv -e
 resp_4_infiniteloop	resp_4_infiniteloop.pcap	resp_4_infiniteloop.out	-vvv -e
+bgp-aigp-oobr		bgp-aigp-oobr.pcap		bgp-aigp-oobr.out		-vvv -e
 
 # RTP tests
 # fuzzed pcap