# HG changeset patch # User Matt Johnston # Date 1614955871 -28800 # Node ID 4983a6bc1f51d39aae506f911540ac58bc5d1896 # Parent c584b5602bd8af8194231d1547af9aaf9c64adbb fuzz: fix crash in newtcpdirect(), don't close the channel too early diff -r c584b5602bd8 -r 4983a6bc1f51 fuzz.h --- 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); diff -r c584b5602bd8 -r 4983a6bc1f51 fuzz/fuzz-common.c --- 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. */ diff -r c584b5602bd8 -r 4983a6bc1f51 netio.c --- 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;