bgpd: clean up some libfuzzer / afl stuff

- Unify the paths a bit more
- Switch to maintaining the peer and not deleting it after each packet;
  this increased coverage in zebra a lot, might help here
- Free processed packets, in libfuzzer case so we dont leak them
- Add check that size is at least BGP_HEADER_SIZE, validate_header()
  assumes it

Signed-off-by: Quentin Young <qlyoung@cumulusnetworks.com>

diff --git a/bgpd/bgp_main.c b/bgpd/bgp_main.c
index 590c2f563700afd335810b0b2f50463894ae505f..96d7006a268c2de387542843cc93e8cffb452a4c 100644 (file)
--- a/bgpd/bgp_main.c
+++ b/bgpd/bgp_main.c
@@ -437,7 +437,13 @@ static bool FuzzingInit(void) {
        return true;
 }
 
-static struct peer *FuzzingCreatePeer()
+/*
+ * Create peer structure that we'll use as the receiver for fuzzed packets.
+ *
+ * state
+ *    BGP FSM state to create the peer in
+ */
+static struct peer *FuzzingCreatePeer(int state)
 {
        union sockunion su;
        sockunion_init(&su);
@@ -447,7 +453,7 @@ static struct peer *FuzzingCreatePeer()
        struct peer *p = peer_create(&su, NULL, bgp_get_default(), 65000, 65001,
                                     0, AFI_IP, SAFI_UNICAST, NULL);
        p->bgp->rpkt_quanta = 1;
-       p->status = Established;
+       p->status = state;
        p->as_type = AS_EXTERNAL;
 
        /* set all flags */
@@ -461,9 +467,7 @@ static struct peer *FuzzingCreatePeer()
        return p;
 }
 
-#ifndef FUZZING_LIBFUZZER
 static struct peer *FuzzingPeer;
-#endif
 
 static bool FuzzingInitialized;
 
@@ -472,6 +476,8 @@ int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size)
        if (!FuzzingInitialized) {
                FuzzingInit();
                FuzzingInitialized = true;
+               /* See comment below */
+               FuzzingPeer = FuzzingCreatePeer(Established);
        }
 
        /*
@@ -484,12 +490,21 @@ int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size)
         * leaks when pointers are nulled that would otherwise be "in-use at
         * exit".
         *
-        * In the libFuzzer case, we need to create it each time, and destroy
-        * it when done.
+        * In the libFuzzer case, we can either create and destroy it each time
+        * to fuzz single packet rx, or we can keep the peer around, which will
+        * fuzz a long lived session. Of course, as state accumulates over
+        * time, memory usage will grow, which imposes some resource
+        * constraints on the fuzzing host. In practice a reasonable server
+        * machine with 64gb of memory or so should be able to fuzz
+        * indefinitely; if bgpd consumes this much memory over time, that
+        * behavior should probably be considered a bug.
         */
        struct peer *p;
 #ifdef FUZZING_LIBFUZZER
-       p = FuzzingCreatePeer();
+       /* For non-persistent mode */
+       // p = FuzzingCreatePeer();
+       /* For persistent mode */
+       p = FuzzingPeer;
 #else
        p = FuzzingPeer;
 #endif /* FUZZING_LIBFUZZER */
@@ -502,10 +517,15 @@ int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size)
         * The actual packet processing code assumes that the size field in the
         * BGP message is correct, and this check is performed by the i/o code,
         * so we need to make sure that remains true for fuzzed input.
-        * */
-       if (!validate_header(p)) {
+        *
+        * Also, validate_header() assumes that there is at least
+        * BGP_HEADER_SIZE bytes in ibuf_work.
+        */
+       if ((size < BGP_HEADER_SIZE) || !validate_header(p)) {
                goto done;
        }
+       fprintf(stderr, "good header\n");
+
 
        int result = 0;
        unsigned char pktbuf[BGP_MAX_PACKET_SIZE];
@@ -528,7 +548,7 @@ int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size)
        }
 
 done:
-       peer_delete(p);
+       //peer_delete(p);
 
        return 0;
 };
@@ -554,7 +574,7 @@ int main(int argc, char **argv)
 
 #ifdef FUZZING
        FuzzingInit();
-       FuzzingPeer = FuzzingCreatePeer();
+       FuzzingPeer = FuzzingCreatePeer(Established);
        FuzzingInitialized = true;
 
 #ifdef __AFL_HAVE_MANUAL_CONTROL

diff --git a/bgpd/bgp_packet.c b/bgpd/bgp_packet.c
index 73ac7eb1ae4401389dea44089bb93e4d16b3c791..a0a31a684ee79c392f102aee385bb6962c12a0d8 100644 (file)
--- a/bgpd/bgp_packet.c
+++ b/bgpd/bgp_packet.c
@@ -2687,13 +2687,13 @@ int bgp_process_packet(struct thread *thread)
                         */
                        assert (!"Message of invalid type received during input processing");
                }
-#ifdef FUZZING
-               return 0;
-#endif
 
                /* delete processed packet */
                stream_free(peer->curr);
                peer->curr = NULL;
+#ifdef FUZZING
+               return 0;
+#endif
                processed++;
 
                /* Update FSM */