Opened 6 years ago

Last modified 3 weeks ago

#8051 assigned defect

Broken check for empty "bridge-ips" line in validate_bridge_stats()

Reported by: asn Owned by:
Priority: Very Low Milestone: Tor: unspecified
Component: Core Tor/Tor Version:
Severity: Normal Keywords: tor-bridge, easy correctness parsing
Cc: isis Actual Points:
Parent ID: Points: 1
Reviewer: Sponsor:

Description

geoip_format_bridge_stats() generates the bridge-stats file contents, and if country_data is NULL then it writes "bridge-ips \n" to the file.

Then when validate_bridge_stats() is called, if it can't find a populated bridge-ips line it tries to match "bridge-ips\n", which should always fail because of the extra whitespace that was added in geoip_format_bridge_stats(). Isn't that right?

The only thing that confuses me is why this hasn't triggered so far. Maybe something in my analysis is incorrect, or the bridge-stats file is always generated after we accept a connection?

Child Tickets

Change History (14)

comment:1 Changed 6 years ago by cypherpunks

"bridge-ips CN=X" and "bridge-ips \n" are the same line for validate_bridge_stats(). well it doesn't count it as empty but count string as valid anyway.

comment:2 in reply to:  1 Changed 6 years ago by asn

Replying to cypherpunks:

"bridge-ips CN=X" and "bridge-ips \n" are the same line for validate_bridge_stats(). well it doesn't count it as empty but count string as valid anyway.

I see what you mean. Then I guess the current check against the _EMPTY_LINE string is useless with the current implementation.

comment:3 Changed 6 years ago by nickm

Keywords: easy added
Milestone: Tor: 0.2.4.x-finalTor: unspecified

Seems a little ugly but basically harmless IIUC. Deferring to "whenever."

comment:4 Changed 4 years ago by isis

Milestone: Tor: unspecifiedTor: 0.2.8.x-final
Owner: set to isis
Points: small
Priority: normaltrivial
Status: newassigned

This is trivially easy; I'll do it, since I'll be touching some of the other bridge code anyway.

comment:5 Changed 3 years ago by nickm

Keywords: TorCoreTeam201605 added

Calling all non-needs_information tickets for May.

comment:6 Changed 3 years ago by isabela

Points: small1

comment:7 Changed 3 years ago by isis

Milestone: Tor: 0.2.8.x-finalTor: 0.2.9.x-final
Severity: Normal

Deferring to 0.2.9 since it doesn't really fix a bug per se.

comment:8 Changed 3 years ago by nickm

Keywords: TorCoreTeam201605 removed

Remove "TorCoreTeam201605" keyword. The time machine is broken.

comment:9 Changed 3 years ago by isis

Milestone: Tor: 0.2.9.x-finalTor: 0.2.???

Defer some of my 029 tickets to ???

comment:10 Changed 3 years ago by teor

Milestone: Tor: 0.2.???Tor: 0.3.???

Milestone renamed

comment:11 Changed 3 years ago by nickm

Keywords: tor-03-unspecified-201612 added
Milestone: Tor: 0.3.???Tor: unspecified

Finally admitting that 0.3.??? was a euphemism for Tor: unspecified all along.

comment:12 Changed 2 years ago by nickm

Keywords: tor-03-unspecified-201612 removed

Remove an old triaging keyword.

comment:13 Changed 2 years ago by nickm

Keywords: correctness parsing added

comment:14 Changed 3 weeks ago by gaba

Cc: isis added
Owner: isis deleted
Note: See TracTickets for help on using tickets.