Wireshark-dev: [Wireshark-dev] bug in packet-bgp.c (Ethereal) and print-bgp.c (tcpdump)

From: Aaron Campbell <aaron@xxxxxxxxx>
Date: Thu, 5 Oct 2006 19:33:52 -0400 (EDT)
[Resending-- was previously sent to ethereal-dev@xxxxxxxxxxxx.]

The expression (BGP_OSPF_RTYPE_EXT ||BGP_OSPF_RTYPE_NSSA) will always
evaluate to 1.  As well, neither of these constants are defined as flag
values, so a bitwise op was probably not intended either.

Commit log for packet-bgp.c in Ethereal shows that this code originated
from Aamer Akhter, and was later ported to tcpdump by Hannes Gredler.  I
have sent them copies of this notice for comment.

Below is my attempt to fix the code in both projects.  Is it correct?
(This is untested, I don't have a trace to test with.)

ethereal patch:
--- packet-bgp.c.orig	Thu Oct  5 19:59:25 2006
+++ packet-bgp.c	Thu Oct  5 20:00:40 2006
@@ -2268,7 +2268,7 @@
 				/* always print E2 even if not external route -- receiving router should ignore */
                                 if ( (tvb_get_guint8(tvb,q+7)) & BGP_OSPF_RTYPE_METRIC_TYPE ) {
                                     junk_gbuf_ptr += g_snprintf(junk_gbuf_ptr, MAX_STR_LEN-(junk_gbuf_ptr-junk_gbuf), " E2");
-                                } else if (tvb_get_guint8(tvb,q+6)==(BGP_OSPF_RTYPE_EXT ||BGP_OSPF_RTYPE_NSSA ) ) {
+                                } else if ( (tvb_get_guint8(tvb,q+6)==BGP_OSPF_RTYPE_EXT) || (tvb_get_guint8(tvb,q+6)==BGP_OSPF_RTYPE_NSSA) ) {
                                     junk_gbuf_ptr += g_snprintf(junk_gbuf_ptr, MAX_STR_LEN-(junk_gbuf_ptr-junk_gbuf), " E1");
                                 } else {
 				    junk_gbuf_ptr += g_snprintf(junk_gbuf_ptr, MAX_STR_LEN-(junk_gbuf_ptr-junk_gbuf), ", no options");

tcpdump patch:
--- print-bgp.c.orig	Thu Oct  5 19:35:04 2006
+++ print-bgp.c	Thu Oct  5 19:45:35 2006
@@ -1508,7 +1508,7 @@
 					  *(tptr+6),
 					  tokbuf, sizeof(tokbuf)),
                                (*(tptr+7) &  BGP_OSPF_RTYPE_METRIC_TYPE) ? "E2" : "",
-                               (*(tptr+6) == (BGP_OSPF_RTYPE_EXT ||BGP_OSPF_RTYPE_NSSA )) ? "E1" : "");
+                               ((*(tptr+6) == BGP_OSPF_RTYPE_EXT) || (*(tptr+6) == BGP_OSPF_RTYPE_NSSA)) ? "E1" : "");
                         break;
                     case BGP_EXT_COM_L2INFO:
                         printf(": %s Control Flags [0x%02x]:MTU %u",

---
Aaron Campbell <aaron@xxxxxxxxx>
Software Engineer, Arbor Networks, Inc.