Opened 9 years ago

Closed 6 years ago

#4645 closed defect (fixed)

Deprecate and remove is_internal_IP

Reported by: rransom Owned by:
Priority: Medium Milestone: Tor: 0.2.5.x-final
Component: Core Tor/Tor Version:
Severity: Keywords: easy tor-client
Cc: Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description


Child Tickets

Attachments (2)

Change History (13)

comment:1 Changed 8 years ago by nickm

Keywords: easy added

Adding "easy" since this is easy to do, but could you explain why?

comment:2 in reply to:  1 Changed 8 years ago by rransom

Replying to nickm:

Adding "easy" since this is easy to do, but could you explain why?

  • is_internal_IP operates on IP addresses stored in a data type which can only handle IPv4 addresses.
  • is_internal_IP operates on IP addresses represented as uint32_t values; this representation risks a byte-order mismatch. (A byte-order mismatch bug involving IPv4 addresses turned up recently in Tor.) Everything which calls is_internal_IP could stand to be audited for this type of bug anyway.

comment:3 Changed 8 years ago by nickm

Keywords: tor-client added

comment:4 Changed 8 years ago by nickm

Component: Tor ClientTor

comment:5 Changed 8 years ago by nickm

Milestone: Tor: 0.2.4.x-finalTor: unspecified

Worth doing, but not urgent.

comment:6 Changed 7 years ago by rl1987

Status: newneeds_review

comment:7 Changed 7 years ago by nickm

Milestone: Tor: unspecifiedTor: 0.2.5.x-final
Status: needs_reviewneeds_revision

Thanks! This needs some tweaks to be mergeable.

First, use tor_addr_from_ipv4n() or tor_addr_from_ipv4h() as appropriate to set up a tor_addr_t from a 32-bit IPv4 address. Use tor_addr_from_in() to set up a tor_addr_t from a struct in_addr.

Second, make sure that the patch builds when "--enable-gcc-warnings" has been passed on the command line. That tells the compiler to enforce some style and correctness checks that we try to make our code follow.

Third, the directory.c change seems weird, given how it converts the string to a 'struct in_addr' and then converts the 'struct in_addr' into a tor_addr_t. Why not use an appropriate conversion function to convert the string to a tor_addr_t directly, if that's the type we want?

comment:8 Changed 7 years ago by rl1987

Status: needs_revisionneeds_review

comment:9 Changed 7 years ago by nickm

It still didn't pass with --enable-gcc-warnings and there was a bug in how it handled "interface_IP".

I have those patches in a branch named "bug4645" in my public tor repository. Have a look?

comment:10 Changed 6 years ago by andrea

I'm okay with this with nickm's changes if he is.

comment:11 Changed 6 years ago by nickm

Resolution: fixed
Status: needs_reviewclosed

great; merged!

Note: See TracTickets for help on using tickets.