bgpctl show mrt crashes when parsing MRT update logs due to NULL ptr deref

classic Classic list List threaded Threaded
1 message Options
Reply | Threaded
Open this post in threaded view
|

bgpctl show mrt crashes when parsing MRT update logs due to NULL ptr deref

Adam Chappell
>Synopsis:      bgpctl show mrt crashes when parsing MRT update logs due to NULL ptr deref
>Category:      system
>Environment:
        System      : OpenBSD 6.7
        Details     : OpenBSD 6.7-current (GENERIC) #324: Tue Jul  7
22:16:16 MDT 2020

[hidden email]:/usr/src/sys/arch/amd64/compile/GENERIC

        Architecture: OpenBSD.amd64
        Machine     : amd64
>Description:

        When attempting to read/parse MRT log files, bgpctl faults
when trying to print BGP route
        attributes. Problem is caused by show_mrt_update() (bgpctl.c)
calling show_attr()
        without a valid parse_result structure. show_attr() uses the
structure for other callers
        to pivot between a '\n' and ';' as a record separator based on
        (parse_result->flags & F_CTL_SSV).

>How-To-Repeat:
        Create simplistic BGP setup which records received updates,
using bgpd.conf
        configuration similar to:

                dump updates in "/var/tmp/updates.log.%I" 3600

        Wait for updates to be recorded to the log, and then inspect using
        "bgpctl show mrt file /var/tmp/dates.log.xx" (replace xx with
current hour)

        Observe crash.

        deraadtix$ bgpctl show mrt file /var/tmp/updates.log.01
        1594604024.513497 89.202.210.101[3257] -> 46.23.92.229[65000]:
size 89 UPDATE
        Segmentation fault (core dumped)

        #0  0x00000a7a6c9f14a6 in show_attr (data=0xa7d09262b9a "",
len=1, res=0x0) at /usr/src/usr.sbin/bgpctl/output.c:820
        820             printf("%c", EOL0(res->flags));

        (gdb) bt
        #0  0x00000a7a6c9f14a6 in show_attr (data=0xa7d09262b9a "",
len=1, res=0x0) at /usr/src/usr.sbin/bgpctl/output.c:820
        #1  0x00000a7a6c9eed7c in show_mrt_update (p=0xa7d09262b97
"@\001\001", len=66) at /usr/src/usr.sbin/bgpctl/bgpctl.c:1594
        #2  0x00000a7a6c9e8e37 in show_mrt_msg (mm=0xa7d4b1af100,
arg=0x7f7fffff8a48) at /usr/src/usr.sbin/bgpctl/bgpctl.c:1677
        #3  0x00000a7a6c9fb754 in mrt_parse (fd=3, p=0xa7a6ca06000,
verbose=1) at /usr/src/usr.sbin/bgpctl/mrtparser.c:199
        #4  0x00000a7a6c9e9b60 in main (argc=4, argv=0x7f7fffff9030)
at /usr/src/usr.sbin/bgpctl/bgpctl.c:147

>Fix:
        Add simple guard to show_attr() until it's clear how
extensively it should support the two output modes
        given that the MRT call chain lacks the context.

--- output.c    Mon Jul 13 11:27:38 2020
+++ output.c.fix        Mon Jul 13 11:26:59 2020
@@ -599,7 +599,10 @@ show_attr(u_char *data, size_t len, struct parse_resul
        u_int16_t        alen, ioff, short_as, afi;
        u_int8_t         flags, type, safi, aid, prefixlen;
        int              i, pos, e2, e4;
+       char             eol;

+       eol = res ? EOL0(res->flags) : '\n';
+
        if (len < 3) {
                warnx("Too short BGP attrbute");
                return;
@@ -817,7 +820,7 @@ show_attr(u_char *data, size_t len, struct parse_resul
                break;
        }
  done:
-       printf("%c", EOL0(res->flags));
+       printf("%c", eol);
 }

 static void