From 55690d05bd8856798d2e6f6b56261629847c068b Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Thu, 11 Apr 2019 18:38:59 -0400 Subject: [PATCH 1/5] Add an assertion to pathbias_get_scale_ratio() This should please coverity, and fix CID 1415723. It didn't understand that networkstatus_get_param() always returns a value between its minimum and maximum values. --- src/feature/client/circpathbias.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/feature/client/circpathbias.c b/src/feature/client/circpathbias.c index 1743ab5a81..e6af649ba7 100644 --- a/src/feature/client/circpathbias.c +++ b/src/feature/client/circpathbias.c @@ -176,6 +176,7 @@ pathbias_get_scale_threshold(const or_options_t *options) static double pathbias_get_scale_ratio(const or_options_t *options) { + (void) options; /* * The scale factor is the denominator for our scaling * of circuit counts for our path bias window. @@ -185,7 +186,8 @@ pathbias_get_scale_ratio(const or_options_t *options) */ int denominator = networkstatus_get_param(NULL, "pb_scalefactor", 2, 2, INT32_MAX); - (void) options; + tor_assert(denominator > 0); + /** * The mult factor is the numerator for our scaling * of circuit counts for our path bias window. It From 96e310911fa14b8a8fb1861d08fbda7abe61eb10 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Thu, 11 Apr 2019 18:38:59 -0400 Subject: [PATCH 2/5] Add an assertion to compute_weighted_bandwidths() This should please coverity, and fix CID 1415722. It didn't understand that networkstatus_get_param() always returns a value between its minimum and maximum values. --- src/feature/nodelist/node_select.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/feature/nodelist/node_select.c b/src/feature/nodelist/node_select.c index e31abb247f..93ddb066d4 100644 --- a/src/feature/nodelist/node_select.c +++ b/src/feature/nodelist/node_select.c @@ -585,6 +585,7 @@ compute_weighted_bandwidths(const smartlist_t *sl, } weight_scale = networkstatus_get_weight_scale_param(NULL); + tor_assert(weight_scale >= 1); if (rule == WEIGHT_FOR_GUARD) { Wg = networkstatus_get_bw_weight(NULL, "Wgg", -1); From 66b07e7ec13feb57d696b920493590d232e62763 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Thu, 11 Apr 2019 18:38:59 -0400 Subject: [PATCH 3/5] Add an assertion to num_ntors_per_tap(). This should please coverity, and fix CID 1415721. It didn't understand that networkstatus_get_param() always returns a value between its minimum and maximum values. --- src/feature/relay/onion_queue.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/feature/relay/onion_queue.c b/src/feature/relay/onion_queue.c index 696905cf5e..c37745cf33 100644 --- a/src/feature/relay/onion_queue.c +++ b/src/feature/relay/onion_queue.c @@ -212,10 +212,12 @@ num_ntors_per_tap(void) #define MIN_NUM_NTORS_PER_TAP 1 #define MAX_NUM_NTORS_PER_TAP 100000 - return networkstatus_get_param(NULL, "NumNTorsPerTAP", - DEFAULT_NUM_NTORS_PER_TAP, - MIN_NUM_NTORS_PER_TAP, - MAX_NUM_NTORS_PER_TAP); + int result = networkstatus_get_param(NULL, "NumNTorsPerTAP", + DEFAULT_NUM_NTORS_PER_TAP, + MIN_NUM_NTORS_PER_TAP, + MAX_NUM_NTORS_PER_TAP); + tor_assert(result > 0); + return result; } /** Choose which onion queue we'll pull from next. If one is empty choose From 48a574604bef9a4fe77d9bb43dfbfc50328e13e0 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Thu, 11 Apr 2019 18:56:02 -0400 Subject: [PATCH 4/5] Remove an extraneous _ from __COVERITY__ We had a typo in this check, so that coverity wasn't taking the right path. Bug not in any released Tor. --- src/lib/math/prob_distr.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lib/math/prob_distr.h b/src/lib/math/prob_distr.h index 2eb935e4a8..8fccf8d015 100644 --- a/src/lib/math/prob_distr.h +++ b/src/lib/math/prob_distr.h @@ -53,7 +53,7 @@ struct dist { * We define this conditionally to suppress false positives from * Coverity, which gets confused by the sizeof business. */ -#ifdef __COVERITY___ +#ifdef __COVERITY__ #define TYPE_CHECK_OBJ(OPS, OBJ, TYPE) 0 #else #define TYPE_CHECK_OBJ(OPS, OBJ, TYPE) \ From 73323460023807259d3cd1dd52d33749ffe53886 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Thu, 11 Apr 2019 18:58:44 -0400 Subject: [PATCH 5/5] Changes file and practracker updates for 30149. --- changes/ticket30149 | 3 +++ scripts/maint/practracker/exceptions.txt | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-) create mode 100644 changes/ticket30149 diff --git a/changes/ticket30149 b/changes/ticket30149 new file mode 100644 index 0000000000..a21687ac2f --- /dev/null +++ b/changes/ticket30149 @@ -0,0 +1,3 @@ + o Code simplification and refactoring: + - Add several assertions in an attempt to fix some Coverity warnings. + Closes ticket 30149. diff --git a/scripts/maint/practracker/exceptions.txt b/scripts/maint/practracker/exceptions.txt index 7d03bf27d6..ecc58012c6 100644 --- a/scripts/maint/practracker/exceptions.txt +++ b/scripts/maint/practracker/exceptions.txt @@ -218,7 +218,7 @@ problem include-count /src/feature/nodelist/networkstatus.c 61 problem function-size /src/feature/nodelist/networkstatus.c:networkstatus_check_consensus_signature() 176 problem function-size /src/feature/nodelist/networkstatus.c:networkstatus_set_current_consensus() 293 problem function-size /src/feature/nodelist/node_select.c:router_pick_directory_server_impl() 123 -problem function-size /src/feature/nodelist/node_select.c:compute_weighted_bandwidths() 205 +problem function-size /src/feature/nodelist/node_select.c:compute_weighted_bandwidths() 206 problem function-size /src/feature/nodelist/node_select.c:router_pick_trusteddirserver_impl() 114 problem function-size /src/feature/nodelist/nodelist.c:compute_frac_paths_available() 193 problem file-size /src/feature/nodelist/routerlist.c 3234