On 2014/03/18 08:57, rivo nurges wrote:
> Hi!
>
> When show_attr reads data length from provided data it reads carbage
> to alen and fails afterwards. This patch fixes the problem by casting
> the data to u_char. While at it I noticed data gets assigned twice.
>
>
> bgpctl.c: /* bad imsg len how can that happen!? */
> bgpctl.c: if (alen > len)
> bgpctl.c: errx(1, "show_attr: bad length");
>
> It can happen:D
>
>
> Example failing prefix with long community list:
> Before:
> BGP routing table entry for 5.44.0.0/20
> 25478 31499 39812
> Nexthop x.y.z.w (via 10.9.0.10) from x.y.z.w (x.y.z.w)
> Origin IGP, metric 4200, localpref 60, weight 0, internal, valid, best
> Last update: 00:12:11 ago
> bgpctl: show_attr: bad length
Haven't found any in my route views yet, though I only got part-way
through when I realised quite how much cpu time session engine was
burning doing a "sh rib d" (hundreds of clock_gettime a second)
and stopped as I worried about it not maintaining sessions and
tripping idle timers.
> Index: usr.sbin/bgpctl/bgpctl.c
> ===================================================================
> RCS file: /OpenBSD/src/usr.sbin/bgpctl/bgpctl.c,v
> retrieving revision 1.173
> diff -u -p -r1.173 bgpctl.c
> --- usr.sbin/bgpctl/bgpctl.c 13 Nov 2013 22:52:41 -0000 1.173
> +++ usr.sbin/bgpctl/bgpctl.c 3 Mar 2014 13:21:23 -0000
> @@ -1346,7 +1346,6 @@ show_attr(void *b, u_int16_t len)
> u_int16_t alen, ioff;
> u_int8_t flags, type;
>
> - data = b;
> if (len < 3)
> errx(1, "show_attr: too short bgp attr");
>
> @@ -1362,7 +1361,7 @@ show_attr(void *b, u_int16_t len)
> data += 4;
> len -= 4;
> } else {
> - alen = data[2];
> + alen = (u_char)data[2];
> data += 3;
> len -= 3;
> }
This is OK sthen@
