On Tue, Feb 14, 2017 at 01:04:40PM +0100, Sebastian Benoit wrote:
> Jonathan Gray([email protected]) 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:
What about the memory leak?
Anyway ok jsg@ to commit your version or something like the below.
Index: parser.c
===================================================================
RCS file: /cvs/src/usr.sbin/bgpctl/parser.c,v
retrieving revision 1.76
diff -u -p -r1.76 parser.c
--- parser.c 13 Feb 2017 14:48:44 -0000 1.76
+++ parser.c 14 Feb 2017 12:16:46 -0000
@@ -1033,21 +1033,26 @@ int
parse_largecommunity(const char *word, struct parse_result *r)
{
struct filter_set *fs;
- char *p = strdup(word);
- char *array[3];
+ char *p, *po = strdup(word);
+ 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;
+ p = po;
+ 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]);
+
+ free(po);
if ((fs = calloc(1, sizeof(struct filter_set))) == NULL)
err(1, NULL);
