#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 _______________________________________________ tor-bugs mailing list tor-bugs@lists.torproject.org https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs