Here is a new version of the diff that fixes the CDP printer in tcpdump.
(https://www.sccs.swarthmore.edu/users/16/mmcconv1/dump/tcpdump-crashes/)

With this change the code does not access outside the packet anymore
but it would be nice if it could be tested on a network with real CDP
(Cisco Discovery Protocol) packets to see if there is any change in the
output. Error checking may have become too aggressive.

Thanks,

Can

Index: print-cdp.c
===================================================================
RCS file: /cvs/src/usr.sbin/tcpdump/print-cdp.c,v
retrieving revision 1.5
diff -u -p -u -p -r1.5 print-cdp.c
--- print-cdp.c 16 Jan 2015 06:40:21 -0000      1.5
+++ print-cdp.c 23 Mar 2016 06:09:31 -0000
@@ -39,7 +39,7 @@
 #include "addrtoname.h"
 #include "extract.h"                   /* must come after interface.h */

-void cdp_print_addr(const u_char * p, int l);
+int cdp_print_addr(const u_char * p, int l);
 void cdp_print_prefixes(const u_char * p, int l);

 /*
@@ -65,7 +65,7 @@ cdp_print(const u_char *p, u_int length,

        while (i < length) {
                if (i + 4 > caplen) {
-                       printf("[!cdp]");
+                       printf("[|cdp]");
                        return;
                }

@@ -75,8 +75,11 @@ cdp_print(const u_char *p, u_int length,
                if (vflag)
                        printf(" %02x/%02x", type, len);

+               if (len < 4)
+                       goto error;
+
                if (i+len > caplen) {
-                       printf("[!cdp]");
+                       printf("[|cdp]");
                        return;
                }

@@ -86,12 +89,15 @@ cdp_print(const u_char *p, u_int length,
                        break;
                case 0x02:
                        printf(" Addr");
-                       cdp_print_addr(p + i + 4, len - 4);
+                       if (cdp_print_addr(p + i + 4, len - 4))
+                               goto error;
                        break;
                case 0x03:
                        printf(" PortID '%.*s'", len - 4, p + i + 4);
                        break;
                case 0x04:
+                       if (len < 8)
+                               goto error;
                        printf(" CAP 0x%02x", (unsigned) p[i+7]);
                        break;
                case 0x05:
@@ -110,9 +116,13 @@ cdp_print(const u_char *p, u_int length,
                        printf(" VTP-Management-Domain '%.*s'", len-4, p+i+4 );
                        break;
                case 0x0a:              /* guess - not documented */
+                       if (len < 6)
+                               goto error;
                        printf(" Native-VLAN-ID %d", (p[i+4]<<8) + p[i+4+1] - 1 
);
                        break;
                case 0x0b:              /* guess - not documented */
+                       if (len < 5)
+                               goto error;
                        printf(" Duplex %s", p[i+4] ? "full": "half" );
                        break;
                default:
@@ -124,42 +134,59 @@ cdp_print(const u_char *p, u_int length,
                        break;
                i += len;
        }
+error:
+       printf("[!cdp]");
 }

-void
+#define CDP_CHECK_ACCESS(p, s) if ((endp - (p)) < (s)) return 1        
+
+int
 cdp_print_addr(const u_char * p, int l)
 {
-       int pl, al, num;
+       int pl, pt, al, num;
        const u_char * endp = p+l;

+       CDP_CHECK_ACCESS(p, 4);
        num = (p[0] << 24) + (p[1]<<16) + (p[2]<<8)+ p[3];
        p+=4;

        printf(" (%d): ", num);

        while(p < endp && num >= 0) {
+               CDP_CHECK_ACCESS(p, 2);
+               pt=*p;
                pl=*(p+1);
                p+=2;

+               CDP_CHECK_ACCESS(p, 3);
                /* special case: IPv4, protocol type=0xcc, addr. length=4 */
                if (pl == 1 && *p == 0xcc && p[1] == 0 && p[2] == 4) {
                        p+=3;

+                       CDP_CHECK_ACCESS(p, 4);
                        printf("IPv4 %d.%d.%d.%d ", p[0], p[1], p[2], p[3]);
                        p+=4;
                } else {        /* generic case: just print raw data */
-                       printf("pt=0x%02x, pl=%d, pb=", *(p-2), pl);
+                       printf("pt=0x%02x, pl=%d, pb=", pt, pl);
+
+                       CDP_CHECK_ACCESS(p, pl);
                        while(pl-- > 0)
                                printf(" %02x", *p++);
+
+                       CDP_CHECK_ACCESS(p, 2);
                        al=(*p << 8) + *(p+1);
                        printf(", al=%d, a=", al);
                        p+=2;
+
+                       CDP_CHECK_ACCESS(p, al);
                        while(al-- > 0)
                                printf(" %02x", *p++);
                }
                printf("  ");
                num--;
        }
+
+       return 0;
 }


@@ -169,7 +196,8 @@ cdp_print_prefixes(const u_char * p, int
        printf(" IPv4 Prefixes (%d):", l/5);

        while (l > 0) {
-               printf(" %d.%d.%d.%d/%d", p[0], p[1], p[2], p[3], p[4] );
+               if (l >= 5)
+                       printf(" %d.%d.%d.%d/%d", p[0], p[1], p[2], p[3], p[4] 
);
                l-=5; p+=5;
        }
 }

Reply via email to