Skip to content
GitLab
Projects Groups Topics Snippets
  • /
  • Help
    • Help
    • Support
    • Community forum
    • Submit feedback
    • Contribute to GitLab
  • Sign in
  • Trac Trac
  • Project information
    • Project information
    • Activity
    • Labels
    • Members
  • Issues 246
    • Issues 246
    • List
    • Boards
    • Service Desk
    • Milestones
  • Monitor
    • Monitor
    • Metrics
    • Incidents
  • Analytics
    • Analytics
    • Value stream
  • Wiki
    • Wiki
  • Activity
  • Create a new issue
  • Issue Boards
Collapse sidebar
  • Legacy
  • TracTrac
  • Issues
  • #22244
Closed (moved) (moved)
Open
Issue created May 13, 2017 by Roger Dingledine@arma

tor_assert(strchr(cp, ':')+2) is never going to do what we want

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;
To upload designs, you'll need to enable LFS and have an admin enable hashed storage. More information
Assignee
Assign to
Time tracking