Index | Thread | Search

From:
Theo Buehler <tb@theobuehler.org>
Subject:
Re: bgpd: fix aspath_inflate for empty ASPATH
To:
tech@openbsd.org
Date:
Fri, 2 Feb 2024 12:47:42 +0100

Download raw body.

Thread
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?

> 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 */
>