Opened 2 years ago

Closed 2 years ago

#22231 closed defect (fixed)

prevent recurrence of CID 1397192

Reported by: catalyst Owned by: catalyst
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:

Description

Coverity found a possible double free in CID 1397192, which dgoulet dismissed as a False Positive. I think I found the logic by which Coverity considered a double free possible. The done block in test_intro_point_registration() has some calls to tt_assert() that can jump backwards if the assertion fails, causing a double free in that unlikely event.

The block that tests hs_circuitmap_free_all() should probably be in a helper function with its own done label that doesn't lead to a double free if the assertion fails.

For reasons I don't understand, it looks like the renames in 6bacc3c7a88509043613d3bc29534c0ecf8803b1 caused Coverity to no longer see this potential double free, even though it looks like it changed nothing relevant.

Child Tickets

Change History (4)

comment:1 Changed 2 years ago by nickm

Agreed; we shouldn't call tt_assert() or any of its kin in a done: block.

comment:2 Changed 2 years ago by catalyst

Status: newneeds_review

comment:3 Changed 2 years ago by nickm

Looks good; tests pass; merging!

comment:4 Changed 2 years ago by nickm

Milestone: Tor: unspecifiedTor: 0.3.1.x-final
Resolution: fixed
Status: needs_reviewclosed
Note: See TracTickets for help on using tickets.