#24488 closed defect (fixed)

Make set_routerstatus_from_routerinfo() set IPv6 unspecified addresses

Reported by: teor Owned by: teor
Priority: Medium Milestone: Tor: 0.3.3.x-final
Component: Core Tor/Tor Version: Tor: 0.2.4.1-alpha
Severity: Normal Keywords: ipv6, code-correctness, review-group-27
Cc: Actual Points: 0.1
Parent ID: Points: 0.1
Reviewer: dgoulet Sponsor: SponsorV-can

Description

set_routerstatus_from_routerinfo() currently sets ipv6_orport to all zeroes. But it should be set to the unspecified IPv6 address.

This is unlikely to cause any bugs in previous Tor versions, but we should fix this for correctness.

This is a bug on commit 6d99c51 in 0.2.4.1-alpha.

Child Tickets

Change History (8)

comment:1 Changed 11 months ago by Sebastian

isn't the unspecified address ::/0 (or all bits equal to zero)?

comment:2 in reply to:  1 Changed 11 months ago by teor

Parent ID: #20916
Status: newneeds_review

(Un-parenting, because this doesn't depend on any of the rest of #20916.)

Replying to Sebastian:

isn't the unspecified address ::/0 (or all bits equal to zero)?

Yes, the unspecified IPv6 address has all bits equal to zero.

But struct tor_addr_t also has a sa_family_t family; member. In this case, it's more correct to have it set to AF_INET6 rather than AF_UNSPEC:

  • it makes for better formatting when we print out the address, and
  • if ipv6_addr is always AF_INET6, it makes it less likely we will introduce bugs.

The initialisation of ipv6_orport is completely redundant, but it's nice to do it explicitly so it's consistent with the if case.

Please see my branch bug24488 on github.

comment:3 Changed 10 months ago by nickm

Keywords: review-group-27 added

comment:4 Changed 10 months ago by nickm

Owner: set to teor
Status: needs_reviewassigned

setting owner

comment:5 Changed 10 months ago by nickm

Status: assignedneeds_review

comment:6 Changed 10 months ago by dgoulet

Reviewer: dgoulet

comment:7 Changed 10 months ago by dgoulet

Status: needs_reviewmerge_ready

lgtm;

comment:8 Changed 10 months ago by nickm

Resolution: fixed
Status: merge_readyclosed

lgtm too; merged to master

Note: See TracTickets for help on using tickets.