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?
>
> Index: parser.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/bgpctl/parser.c,v
> retrieving revision 1.76
> diff -u -p -u -p -r1.76 parser.c
> --- parser.c 13 Feb 2017 14:48:44 -0000 1.76
> +++ parser.c 14 Feb 2017 08:59:43 -0000
> @@ -1034,16 +1034,21 @@ parse_largecommunity(const char *word, s
> {
> struct filter_set *fs;
> char *p = strdup(word);
> - char *array[3];
> + char *array[3] = { NULL, NULL, NULL };
> int64_t as, ld1, ld2;
> int i = 0;
>
> while (p != NULL) {
> + if (i > 2)
> + errx(1, "Invalid Large-Community syntax");
> array[i++] = p;
> p = strchr(p, ':');
> if (p)
> *p++ = 0;
> }
> +
> + if (!(array[0] && array[1] && array[2]))
> + errx(1, "Invalid Large-Community syntax");
>
> as = getlargecommunity(array[0]);
> ld1 = getlargecommunity(array[1]);
>
>
> --
> The four building blocks of the universe are fire, water, gravel and
> vinyl.
> -- Dave Barry
>