Opened 2 years ago

Closed 2 years ago

#26072 closed defect (fixed)

Malformed connected cell closes connection but code continues

Reported by: mikeperry Owned by: nickm
Priority: Medium Milestone: Tor: 0.3.4.x-final
Component: Core Tor/Tor Version:
Severity: Normal Keywords: 034-must spec-conformance 029-backport 031-backport 032-backport 033-backport
Cc: Actual Points:
Parent ID: Points:
Reviewer: mikeperry Sponsor:


In connection_edge_process_relay_cell_not_open(), there is a clause that closes the connection if the connected cell is malformed. However, it does not return from the function. Every other clause where the connection is closed does return.

This looks like a bug. I couldn't immediately find any issues with this, though. Perhaps an assert if the connection gets marked twice..

Child Tickets

Change History (7)

comment:1 Changed 2 years ago by nickm

Keywords: 034-must spec-conformance added
Milestone: Tor: 0.3.4.x-final

comment:2 Changed 2 years ago by nickm

Keywords: 029-backport 031-backport 032-backport 033-backport added
Owner: set to nickm
Status: newaccepted

Please review my branch bug26072_029. It's a one-line fix.

comment:3 Changed 2 years ago by nickm

Status: acceptedneeds_review

comment:4 Changed 2 years ago by nickm

Reviewer: mikeperry

comment:5 Changed 2 years ago by mikeperry

Status: needs_reviewmerge_ready

This looks good.

One question - under what circumstances might a connected cell be properly formed but still AF_UNSPEC? Are there any? That is what made me hesitate before just making this patch.

comment:6 Changed 2 years ago by nickm

Looking at connected_cell_parse, it's possible for the connected cell just to be empty. If it is, then the relay provided neither an address nor a TTL, and the address was left as unspec.

comment:7 Changed 2 years ago by nickm

Resolution: fixed
Status: merge_readyclosed

Merged to 0.2.9 and forward!

Note: See TracTickets for help on using tickets.