From 16b0b8f5006f6dc7bf0ae6d63399c6d3c867766a Mon Sep 17 00:00:00 2001
From: Quentin Young <qlyoung@cumulusnetworks.com>
Date: Fri, 17 Jan 2020 10:23:24 -0500
Subject: [PATCH] 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>
---
 bgpd/bgp_main.c   | 42 +++++++++++++++++++++++++++++++-----------
 bgpd/bgp_packet.c |  6 +++---
 2 files changed, 34 insertions(+), 14 deletions(-)

diff --git a/bgpd/bgp_main.c b/bgpd/bgp_main.c
index 590c2f5637..96d7006a26 100644
--- 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 73ac7eb1ae..a0a31a684e 100644
--- 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 */
-- 
2.39.5