From: Claudio Jeker Subject: Re: bgpd: fix aspath_inflate for empty ASPATH To: Theo Buehler Cc: tech@openbsd.org Date: Fri, 2 Feb 2024 17:16:38 +0100 On Fri, Feb 02, 2024 at 12:47:42PM +0100, Theo Buehler wrote: > On Fri, Feb 02, 2024 at 12:42:59PM +0100, Claudio Jeker wrote: > > This bug was found by the mrt regress test. The problem is that > > an empty ASPATH has lenght 0 and ibuf_open(0) fails (currently this is the > > case but I had this changed in my work tree so I did not notice that). > > ok > > I meant to point this out during review, but I think I forgot: do we > want to overflow check this multiplication or (entirely for completeness) > or do we not care since we will error after having done (probably a lot) > of work? I don't think an overflow of the size_t is possible. The length of the attribute is limited to a uint16_t in rde_attr_parse() and so this can not overflow. Btw. the same is true for mrt_extract_attr() which it the only other code calling aspath_inflate. > > Use a simple hack to allocate an extra byte so ibuf_open() succeeds. > > With this the mrt regress is happy again. If we change the ibuf_open() > > behaviour this can be reverted. > > cool. > > > > > -- > > :wq Claudio > > > > Index: util.c > > =================================================================== > > RCS file: /cvs/src/usr.sbin/bgpd/util.c,v > > diff -u -p -r1.80 util.c > > --- util.c 30 Jan 2024 13:50:09 -0000 1.80 > > +++ util.c 2 Feb 2024 11:38:27 -0000 > > @@ -547,8 +547,12 @@ aspath_inflate(struct ibuf *in) > > uint16_t short_as; > > uint8_t seg_type, seg_len; > > > > - /* allocate enough space for the worst case */ > > - if ((out = ibuf_open(ibuf_size(in) * 2)) == NULL) > > + /* > > + * allocate enough space for the worst case, > > + * add 1 byte for the empty ASPATH case since we can't > > + * allocate an ibuf of 0 length. > > + */ > > + if ((out = ibuf_open(ibuf_size(in) * 2 + 1)) == NULL) > > return (NULL); > > > > /* then copy the aspath */ > > > -- :wq Claudio