changeset 1801:4983a6bc1f51 default tip main master

fuzz: fix crash in newtcpdirect(), don't close the channel too early
author Matt Johnston <matt@ucc.asn.au>
date Fri, 05 Mar 2021 22:51:11 +0800
parents c584b5602bd8
children
files fuzz.h fuzz/fuzz-common.c netio.c
diffstat 3 files changed, 7 insertions(+), 31 deletions(-) [+]
line wrap: on
line diff
--- a/fuzz.h	Fri Mar 05 21:13:20 2021 +0800
+++ b/fuzz.h	Fri Mar 05 22:51:11 2021 +0800
@@ -37,11 +37,6 @@
 void fuzz_seed(const unsigned char* dat, unsigned int len);
 void fuzz_svr_hook_preloop(void);
 
-typedef void(*connect_callback)(int result, int sock, void* data, const char* errstring);
-struct dropbear_progress_connection *fuzz_connect_remote(const char* remotehost, const char* remoteport,
-    connect_callback cb, void* cb_data,
-    const char* bind_address, const char* bind_port);
-
 int fuzz_dropbear_listen(const char* address, const char* port,
         int *socks, unsigned int sockcount, char **errstring, int *maxfd);
 
--- a/fuzz/fuzz-common.c	Fri Mar 05 21:13:20 2021 +0800
+++ b/fuzz/fuzz-common.c	Fri Mar 05 22:51:11 2021 +0800
@@ -235,26 +235,6 @@
 }
 
 
-struct dropbear_progress_connection *fuzz_connect_remote(const char* UNUSED(remotehost), const char* UNUSED(remoteport),
-    connect_callback cb, void* cb_data, 
-    const char* UNUSED(bind_address), const char* UNUSED(bind_port)) {
-    /* This replacement for connect_remote() has slightly different semantics
-    to the real thing. It should probably be replaced with something more sophisticated.
-    It calls the callback cb() immediately rather than
-    in a future session loop iteration with set_connect_fds()/handle_connect_fds().
-    This could cause problems depending on how connect_remote() is used. In particular
-    the callback can close a channel - that can cause use-after-free. */
-    char r;
-    genrandom((void*)&r, 1);
-    if (r & 1) {
-        int sock = wrapfd_new_dummy();
-        cb(DROPBEAR_SUCCESS, sock, cb_data, NULL);
-    } else {
-        cb(DROPBEAR_FAILURE, -1, cb_data, "errorstring");
-    }
-    return NULL;
-}
-
 /* Fake dropbear_listen, always returns failure for now.
 TODO make it sometimes return success with wrapfd_new_dummy() sockets.
 Making the listeners fake a new incoming connection will be harder. */
--- a/netio.c	Fri Mar 05 21:13:20 2021 +0800
+++ b/netio.c	Fri Mar 05 22:51:11 2021 +0800
@@ -179,12 +179,6 @@
 	int err;
 	struct addrinfo hints;
 
-#if DROPBEAR_FUZZ
-	if (fuzz.fuzzing) {
-		return fuzz_connect_remote(remotehost, remoteport, cb, cb_data, bind_address, bind_port);
-	}
-#endif
-
 	c = m_malloc(sizeof(*c));
 	c->remotehost = m_strdup(remotehost);
 	c->remoteport = m_strdup(remoteport);
@@ -194,6 +188,13 @@
 
 	list_append(&ses.conn_pending, c);
 
+#if DROPBEAR_FUZZ
+	if (fuzz.fuzzing) {
+		c->errstring = m_strdup("fuzzing connect_remote always fails");
+		return c;
+	}
+#endif
+
 	memset(&hints, 0, sizeof(hints));
 	hints.ai_socktype = SOCK_STREAM;
 	hints.ai_family = AF_UNSPEC;