Opened 3 years ago

Closed 3 years ago

#22244 closed defect (fixed)

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

Reported by: arma Owned by:
Priority: Medium Milestone: Tor: 0.3.1.x-final
Component: Core Tor/Tor Version:
Severity: Normal Keywords:
Cc: Actual Points:
Parent ID: Points:
Reviewer: Sponsor:


Here's the code in src/or/dns.c:

    const char *err = strchr(cp, ':')+2;

Andrey Karpov points out in 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:
and then mwenge noticed the underlying bug and nickm fixed it here:
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, ':');
    const char *err = colon+2;

Child Tickets

Change History (1)

comment:1 Changed 3 years ago by nickm

Resolution: fixed
Status: newclosed

in master 0e348720fc3a4d fixes this; not planning to backport. I took Roger's solution above, since it looks fine to me

Note: See TracTickets for help on using tickets.