[tor-bugs] #22244 [Core Tor/Tor]: tor_assert(strchr(cp, ':')+2) is never going to do what we want

[Tor Bug Tracker & Wiki]

#22244: tor_assert(strchr(cp, ':')+2) is never going to do what we want
------------------------------+--------------------------------
     Reporter:  arma          |      Owner:
         Type:  defect        |     Status:  new
     Priority:  Medium        |  Milestone:  Tor: 0.3.1.x-final
    Component:  Core Tor/Tor  |    Version:
     Severity:  Normal        |   Keywords:
Actual Points:                |  Parent ID:
       Points:                |   Reviewer:
      Sponsor:                |
------------------------------+--------------------------------
 Here's the code in src/or/dns.c:
 {{{
     const char *err = strchr(cp, ':')+2;
     tor_assert(err);
 }}}
 Andrey Karpov points out in https://www.viva64.com/en/b/0507/ that that
 code is never going to do what we want: even if strchr returns NULL, err
 will still be 2.

 The history of that code is actually kind of exciting. It used to be
 {{{strchr(cp, ':'+2)}}}, which Veracode freaked out about:
 https://lists.torproject.org/pipermail/tor-
 commits/2008-February/008431.html
 and then mwenge noticed the underlying bug and nickm fixed it here:
 https://lists.torproject.org/pipermail/tor-
 commits/2008-February/008530.html
 but we left the tor_assert line in place.

 What's the right fix? We could simply take out the assert, because hey
 it's never been a problem. Or we could break it out into something like
 {{{
     const char *colon = strchr(cp, ':');
     tor_assert(colon);
     const char *err = colon+2;
 }}}

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