[tor-bugs] #21971 [Core Tor/Tor]: Coverity issues in HS circuitmap unittests

[Tor Bug Tracker & Wiki]

#21971: Coverity issues in HS circuitmap unittests
------------------------------+-------------------------------------
     Reporter:  asn           |      Owner:
         Type:  defect        |     Status:  new
     Priority:  Medium        |  Milestone:  Tor: 0.3.1.x-final
    Component:  Core Tor/Tor  |    Version:
     Severity:  Normal        |   Keywords:  tor-hs prop224 coverity
Actual Points:                |  Parent ID:
       Points:  0.3           |   Reviewer:
      Sponsor:  SponsorR-can  |
------------------------------+-------------------------------------
 We got two reports from coverity about the HS circuitmap unittests
 (#21889) which were merged in 0.3.1.

 First one:
 {{{
 *** CID 1405129:    (NEGATIVE_RETURNS)
 /src/test/test_hs_intropoint.c: 780 in test_received_introduce1_handling()
 774       }
 775
 776       /* Valid case. */
 777       {
 778         cell = helper_create_introduce1_cell();
 779         ssize_t request_len = trn_cell_introduce1_encoded_len(cell);
 >>>     CID 1405129:    (NEGATIVE_RETURNS)
 >>>     Assigning: unsigned variable "print_" = "print1_".
 780         tt_size_op(request_len, OP_GT, 0);
 }}}
 which is caused because we use `tt_size_op` to compare a `ssize_t`.


 ----

 {{{
 *** CID 1405130:    (REVERSE_INULL)
 /src/test/test_circuitlist.c: 440 in test_hs_circuitmap_isolation()
 434          * that token. */
 435         tt_ptr_op(circ4, OP_EQ,
 436                   hs_circuitmap_get_intro_circ_v2_service_side(tok2));
 437       }
 438
 439      done:
 >>>     CID 1405130:    (REVERSE_INULL)
 >>>     Null-checking "circ1" suggests that it may be null, but it has
 already been dereferenced on all paths leading to the check.
 440       if (circ1)
 441         circuit_free(TO_CIRCUIT(circ1));
 }}}

 which I think is caused because we NULL check `circ1`, even tho previous
 code is dereferencing it without any checks. Not sure what the right fix
 is here; perhaps we dont really need to NULL check it, and we can add a
 `tt_assert(circ1)` on the top as well.

--
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/21971>
Tor Bug Tracker & Wiki <https://trac.torproject.org/>
The Tor Project: anonymity online