[Author Prev][Author Next][Thread Prev][Thread Next][Author Index][Thread Index]

Re: [tor-bugs] #22490 [Core Tor/Tor]: Stop using GeoIP country after buf has gone out of scope

#22490: Stop using GeoIP country after buf has gone out of scope
-------------------------------------------------+-------------------------
 Reporter:  teor                                 |          Owner:
     Type:  defect                               |         Status:
                                                 |  merge_ready
 Priority:  Medium                               |      Milestone:  Tor:
                                                 |  0.3.1.x-final
Component:  Core Tor/Tor                         |        Version:
 Severity:  Normal                               |     Resolution:
 Keywords:  memory-safety 024-backport           |  Actual Points:  0
  025-backport 026-backport 027-backport         |
  028-backport 029-backport 030-backport         |
Parent ID:                                       |         Points:  0.2
 Reviewer:                                       |        Sponsor:
-------------------------------------------------+-------------------------
Changes (by teor):

 * status:  needs_review => merge_ready


Comment:

 This fix looks good, and I think it should be backported to 0.2.4.

 I also don't think it needs a new release, because we're not actually
 overwriting country with the geoip_add_entry() stack. But we get within 6
 bytes of overwriting it.

 If the IPv6 addresses were both short, then the geoip_add_entry() stack
 could overwrite `country`, and leak pointer values: it allocates 3
 pointers on the stack (24 bytes on x86_64, 12 bytes on i386).

 But the shortest line is 32 characters (did we have shorter lines in
 earlier files?), which means that the earliest country could start is at
 buf+30.

 Ah, but wait: pointers must be aligned on word boundaries, and we've added
 a call to geoip_add_entry() to the stack, as well as any registers we've
 saved, and any stack smashing protection, any any other compiler
 bookkeeping.

 But I haven't been able to reproduce it locally using clang, macOS,
 x86_64, and my diagnostic branch `assert-country` with the command
 `src/or/tor DisableNetwork 1 GeoIPv6File src/config/geoip6`, even with two
 2-character IPv6 addresses.

 So I think this could trigger in rare circumstances, but common configs
 are safe.

 But remember those weird countries we were getting in some relay
 descriptors?
 Did we ever track that down?
 This could be it.

--
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/22490#comment:5>
Tor Bug Tracker & Wiki <https://trac.torproject.org/>
The Tor Project: anonymity online
_______________________________________________
tor-bugs mailing list
tor-bugs@xxxxxxxxxxxxxxxxxxxx
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs