From 68792f77e51f84d0fb6758ef9491a70570ac9a53 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Fri, 28 Jun 2019 12:21:49 -0400 Subject: [PATCH 1/4] Fix a few coverity unitinitialzed-value warnings in the unit tests. Coverity can't see that it is not in fact going to read uninitialized memory here, so we initialize these values unconditionally. Bugfix on 0.4.0.1-alpha. --- src/test/test_btrack.c | 4 ++++ src/test/test_controller_events.c | 3 +++ 2 files changed, 7 insertions(+) diff --git a/src/test/test_btrack.c b/src/test/test_btrack.c index 9e5d0d0723..21e88a57b6 100644 --- a/src/test/test_btrack.c +++ b/src/test/test_btrack.c @@ -44,6 +44,8 @@ test_btrack_launch(void *arg) { orconn_state_msg_t conn; ocirc_chan_msg_t circ; + memset(&conn, 0, sizeof(conn)); + memset(&circ, 0, sizeof(circ)); (void)arg; conn.gid = 1; @@ -93,6 +95,8 @@ test_btrack_delete(void *arg) { orconn_state_msg_t state; orconn_status_msg_t status; + memset(&state, 0, sizeof(state)); + memset(&status, 0, sizeof(status)); (void)arg; state.gid = 1; diff --git a/src/test/test_controller_events.c b/src/test/test_controller_events.c index a8967bba50..9fb2bc7256 100644 --- a/src/test/test_controller_events.c +++ b/src/test/test_controller_events.c @@ -429,6 +429,7 @@ static void test_cntev_orconn_state(void *arg) { orconn_state_msg_t conn; + memset(&conn, 0, sizeof(conn)); (void)arg; MOCK(queue_control_event_string, mock_queue_control_event_string); @@ -468,6 +469,7 @@ static void test_cntev_orconn_state_pt(void *arg) { orconn_state_msg_t conn; + memset(&conn, 0, sizeof(conn)); (void)arg; MOCK(queue_control_event_string, mock_queue_control_event_string); @@ -503,6 +505,7 @@ static void test_cntev_orconn_state_proxy(void *arg) { orconn_state_msg_t conn; + memset(&conn, 0, sizeof(conn)); (void)arg; MOCK(queue_control_event_string, mock_queue_control_event_string); From 5fa2b322005d1860d39e420cb6d3ed25f5073389 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Fri, 28 Jun 2019 12:24:26 -0400 Subject: [PATCH 2/4] Coverity: fix test issues with always-present 'service' var. Coverity is worried that we check "service" at the end of these test functions, since it doesn't see any way to reach the cleanup code without having first dereferenced the variable. Removing the check would be unwise in this case: instead we add a tt_assert check before using "service" so that coverity thinks that the check is doing something useful. Bugfix on 0.3.2.1-alpha. --- src/test/test_hs_common.c | 1 + src/test/test_hs_service.c | 1 + 2 files changed, 2 insertions(+) diff --git a/src/test/test_hs_common.c b/src/test/test_hs_common.c index abded6021e..de3f7e04f7 100644 --- a/src/test/test_hs_common.c +++ b/src/test/test_hs_common.c @@ -502,6 +502,7 @@ test_desc_reupload_logic(void *arg) pubkey_hex, strlen(pubkey_hex)); hs_build_address(&pubkey, HS_VERSION_THREE, onion_addr); service = tor_malloc_zero(sizeof(hs_service_t)); + tt_assert(service); memcpy(service->onion_address, onion_addr, sizeof(service->onion_address)); ed25519_secret_key_generate(&service->keys.identity_sk, 0); ed25519_public_key_generate(&service->keys.identity_pk, diff --git a/src/test/test_hs_service.c b/src/test/test_hs_service.c index a303f10411..2e4be4e295 100644 --- a/src/test/test_hs_service.c +++ b/src/test/test_hs_service.c @@ -1265,6 +1265,7 @@ test_service_event(void *arg) /* Set a service for this circuit. */ service = helper_create_service(); + tt_assert(service); ed25519_pubkey_copy(&circ->hs_ident->identity_pk, &service->keys.identity_pk); From ea154a6108bae597cb37e6bc53036b6dd2ed6187 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Fri, 28 Jun 2019 12:27:51 -0400 Subject: [PATCH 3/4] Coverity: fix memory leak on error in test function. The function make_intro_from_plaintext() in test_introduce.c would leak memory if we ever hit a failure from our underlying crypto functions. This kind of failure should be impossible, but it's best to be safe here. Bugfix on 0.2.4.1-alpha. --- src/test/test_introduce.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/test/test_introduce.c b/src/test/test_introduce.c index 4a6d90d97e..104e973b1f 100644 --- a/src/test/test_introduce.c +++ b/src/test/test_introduce.c @@ -383,8 +383,10 @@ make_intro_from_plaintext( /* Output the cell */ *cell_out = cell; + cell = NULL; done: + tor_free(cell); return cell_len; } @@ -535,4 +537,3 @@ struct testcase_t introduce_tests[] = { INTRODUCE_LEGACY(late_parse_v3), END_OF_TESTCASES }; - From 75ea7514e1e87124bf250b36a1a1c243b6b0cea0 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Fri, 28 Jun 2019 12:36:12 -0400 Subject: [PATCH 4/4] Add a changes file for coverity test fixes of 31030. --- changes/ticket31030 | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 changes/ticket31030 diff --git a/changes/ticket31030 b/changes/ticket31030 new file mode 100644 index 0000000000..4d99323b4e --- /dev/null +++ b/changes/ticket31030 @@ -0,0 +1,3 @@ + o Minor bugfixes (coverity, tests): + - Fix several coverity warnings from our unit tests. Fixes bug 31030; + bugfix on 0.2.4.1-alpha, 0.3.2.1-alpha, and 0.4.0.1-alpha.