hs: Fix an underflow in rend_service_intro_has_opened()

In rend_service_intro_has_opened(), this is subject to a possible underflow
because of how the if() casts the results. In the case where the expiring
nodes list length is bigger than the number of IP circuits, we end up in the
following situation where the result will be cast to an unsigned int. For
instance, "5 - 6" is actually a BIG number.

Ultimately leading to closing IP circuits in a non stop loop.

Partially fixes #21302.

Signed-off-by: David Goulet <dgoulet@torproject.org>
This commit is contained in:
David Goulet 2017-02-01 11:22:17 -05:00 committed by Nick Mathewson
parent cc0342a2ae
commit 8573d99470

View File

@ -3227,6 +3227,7 @@ rend_service_intro_has_opened(origin_circuit_t *circuit)
rend_service_t *service; rend_service_t *service;
char buf[RELAY_PAYLOAD_SIZE]; char buf[RELAY_PAYLOAD_SIZE];
char serviceid[REND_SERVICE_ID_LEN_BASE32+1]; char serviceid[REND_SERVICE_ID_LEN_BASE32+1];
unsigned int expiring_nodes_len, num_ip_circuits, valid_ip_circuits = 0;
int reason = END_CIRC_REASON_TORPROTOCOL; int reason = END_CIRC_REASON_TORPROTOCOL;
const char *rend_pk_digest; const char *rend_pk_digest;
@ -3248,13 +3249,22 @@ rend_service_intro_has_opened(origin_circuit_t *circuit)
goto err; goto err;
} }
/* Take the current amount of expiring nodes and the current amount of IP
* circuits and compute how many valid IP circuits we have. */
expiring_nodes_len = (unsigned int) smartlist_len(service->expiring_nodes);
num_ip_circuits = count_intro_point_circuits(service);
/* Let's avoid an underflow. The valid_ip_circuits is initialized to 0 in
* case this condition turns out false because it means that all circuits
* are expiring so we need to keep this circuit. */
if (num_ip_circuits > expiring_nodes_len) {
valid_ip_circuits = num_ip_circuits - expiring_nodes_len;
}
/* If we already have enough introduction circuits for this service, /* If we already have enough introduction circuits for this service,
* redefine this one as a general circuit or close it, depending. * redefine this one as a general circuit or close it, depending.
* Substract the amount of expiring nodes here since the circuits are * Substract the amount of expiring nodes here because the circuits are
* still opened. */ * still opened. */
if ((count_intro_point_circuits(service) - if (valid_ip_circuits > service->n_intro_points_wanted) {
smartlist_len(service->expiring_nodes)) >
service->n_intro_points_wanted) {
const or_options_t *options = get_options(); const or_options_t *options = get_options();
/* Remove the intro point associated with this circuit, it's being /* Remove the intro point associated with this circuit, it's being
* repurposed or closed thus cleanup memory. */ * repurposed or closed thus cleanup memory. */