Re: bgpctl large community parsing issues

[Sebastian Benoit]

Jonathan Gray(j...@jsg.id.au) on 2017.02.14 20:23:51 +1100:
> On Tue, Feb 14, 2017 at 10:00:17AM +0100, Peter Hessler wrote:
> > On 2017 Feb 14 (Tue) at 15:39:45 +1100 (+1100), Jonathan Gray wrote:
> > :The bgpctl parser for large communities makes invalid assumptions about
> > :the string passed into parse_largecommunity() and also seems to leak
> > :the memory returned by strdup in the same function.
> > :
> > :(gdb) run show rib large-community 1:1
> > :Starting program: /usr/obj/usr.sbin/bgpctl/bgpctl show rib large-community 
> > 1:1
> > :
> > :Program received signal SIGBUS, Bus error.
> > :getlargecommunity (
> > :    s=0x9940329f0b9b6d6e <Address 0x9940329f0b9b6d6e out of bounds>)
> > :    at /usr/src/usr.sbin/bgpctl/parser.c:1022
> > :1022            if (strcmp(s, "*") == 0)
> > :(gdb) p s
> > :$1 = 0x9940329f0b9b6d6e <Address 0x9940329f0b9b6d6e out of bounds>
> > :(gdb) bt
> > :#0  getlargecommunity (
> > :    s=0x9940329f0b9b6d6e <Address 0x9940329f0b9b6d6e out of bounds>)
> > :    at /usr/src/usr.sbin/bgpctl/parser.c:1022
> > 
> > Easy fix.  Pre-initialize the array to NULL, then check if they are set.
> > Also, check to see if we got too many ':'.
> > 
> > OK?
> 
> Would strsep be a better fit here?


        while ((p != NULL) && (i < 3)) {
                val = strsep(&p, ":");
                if (*val == '\0')
                        errx(1, "Invalid Large-Community syntax");
                array[i++] = val;
        }

        if ((p != NULL) || !(array[0] && array[1] && array[2]))
                errx(1, "Invalid Large-Community syntax");


and actually the if (*val == '\0') ...
can be removed, because that will just result in array[x] being NULL:

diff --git usr.sbin/bgpctl/parser.c usr.sbin/bgpctl/parser.c
index f3a569bedc1..a2115a77b59 100644
--- usr.sbin/bgpctl/parser.c
+++ usr.sbin/bgpctl/parser.c
@@ -1034,17 +1034,19 @@ parse_largecommunity(const char *word, struct 
parse_result *r)
 {
        struct filter_set *fs;
        char            *p = strdup(word);
-       char            *array[3];
+       char            *array[3] = { NULL, NULL, NULL };
+       char            *val;
        int64_t          as, ld1, ld2;
        int              i = 0;
 
-       while (p != NULL) {
-               array[i++] = p;
-               p = strchr(p, ':');
-               if (p)
-                       *p++ = 0;
+       while ((p != NULL) && (i < 3)) {
+               val = strsep(&p, ":");
+               array[i++] = val;
        }
 
+       if ((p != NULL) || !(array[0] && array[1] && array[2]))
+               errx(1, "Invalid Large-Community syntax");
+
        as   = getlargecommunity(array[0]);
        ld1  = getlargecommunity(array[1]);
        ld2  = getlargecommunity(array[2]);