From 6439d322543964b5c4cf3f7adce9ff33b4cd7cfd Mon Sep 17 00:00:00 2001 From: Luca Boccassi Date: Fri, 15 May 2020 14:10:43 +0100 Subject: [PATCH 1/2] Problem: fuzz tests do not check that legitimate clients work Solution: add normal client sockets and bounce at the same time as the mock client --- tests/test_bind_curve_fuzzer.cpp | 25 ++++++++++++++++++++++++- tests/test_bind_null_fuzzer.cpp | 10 +++++++++- 2 files changed, 33 insertions(+), 2 deletions(-) diff --git a/tests/test_bind_curve_fuzzer.cpp b/tests/test_bind_curve_fuzzer.cpp index 5b4d5c9c..643a3723 100644 --- a/tests/test_bind_curve_fuzzer.cpp +++ b/tests/test_bind_curve_fuzzer.cpp @@ -42,6 +42,10 @@ extern "C" int LLVMFuzzerTestOneInput (const uint8_t *data, size_t size) { const char *fixed_client_public = "{{k*81)yMWEF{/BxdMd[5RL^qRFxBgoL<8m.D^KD"; + const char *fixed_client_secret = + "N?Gmik8R[2ACw{b7*[-$S6[4}aO#?DB?#=C"; const char *fixed_server_secret = "T}t5GLq%&Qm1)y3ywu-}pY3KEA//{^Ut!M1ut+B4"; void *handler; @@ -86,8 +90,27 @@ extern "C" int LLVMFuzzerTestOneInput (const uint8_t *data, size_t size) sent = send (client, (const char *) data, size, MSG_NOSIGNAL); msleep (250); - close (client); + // Drain the queue, if any + zmq_msg_t msg; + zmq_msg_init (&msg); + while (-1 != zmq_msg_recv (&msg, server, ZMQ_DONTWAIT)) { + zmq_msg_close (&msg); + zmq_msg_init (&msg); + } + // A well-behaved client should work while the malformed data from the other + // is being received + curve_client_data_t curve_client_data = { + fixed_server_public, fixed_client_public, fixed_client_secret}; + void *client_mon; + void *client_good = create_and_connect_client ( + my_endpoint, socket_config_curve_client, &curve_client_data, &client_mon); + + bounce (server, client_good); + + close (client); + test_context_socket_close_zero_linger (client_good); + test_context_socket_close_zero_linger (client_mon); shutdown_context_and_server_side (zap_thread, server, server_mon, handler); teardown_test_context (); diff --git a/tests/test_bind_null_fuzzer.cpp b/tests/test_bind_null_fuzzer.cpp index 291b9533..11d289f2 100644 --- a/tests/test_bind_null_fuzzer.cpp +++ b/tests/test_bind_null_fuzzer.cpp @@ -49,6 +49,11 @@ extern "C" int LLVMFuzzerTestOneInput (const uint8_t *data, size_t size) bind_loopback_ipv4 (server, my_endpoint, sizeof (my_endpoint)); fd_t client = connect_socket (my_endpoint); + void *client_good = test_context_socket (ZMQ_SUB); + TEST_ASSERT_SUCCESS_ERRNO ( + zmq_setsockopt (client_good, ZMQ_SUBSCRIBE, "", 0)); + TEST_ASSERT_SUCCESS_ERRNO (zmq_connect (client_good, my_endpoint)); + // If there is not enough data for a full greeting, just send what we can // Otherwise send greeting first, as expected by the protocol uint8_t buf[64]; @@ -64,8 +69,11 @@ extern "C" int LLVMFuzzerTestOneInput (const uint8_t *data, size_t size) sent = send (client, (const char *) data, size, MSG_NOSIGNAL); msleep (250); - close (client); + TEST_ASSERT_EQUAL_INT (6, zmq_send_const (server, "HELLO", 6, 0)); + TEST_ASSERT_EQUAL_INT (6, zmq_recv (client_good, buf, 6, 0)); + close (client); + test_context_socket_close_zero_linger (client_good); test_context_socket_close_zero_linger (server); teardown_test_context (); From 6815138501b9f2a69e807bc3527d93583e633233 Mon Sep 17 00:00:00 2001 From: Luca Boccassi Date: Fri, 15 May 2020 17:07:48 +0100 Subject: [PATCH 2/2] Problem: unfinished message can be leaked by client pipe When a pipe processes a delimiter and is already not in active state but still has an unfinished message, the message is leaked. Solution: issue a rollback before losing the reference to the pipe. --- src/pipe.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/pipe.cpp b/src/pipe.cpp index 69d5e4f6..b391b6ea 100644 --- a/src/pipe.cpp +++ b/src/pipe.cpp @@ -506,6 +506,7 @@ void zmq::pipe_t::process_delimiter () if (_state == active) _state = delimiter_received; else { + rollback (); _out_pipe = NULL; send_pipe_term_ack (_peer); _state = term_ack_sent;