Opened 16 years ago
Closed 15 years ago
#4579 closed defect (bug) (fixed)
IPv6 IPs
Reported by: |
|
Owned by: |
|
---|---|---|---|
Milestone: | 2.5 | Priority: | low |
Severity: | minor | Version: | |
Component: | General | Keywords: | has-patch ipv6 tested dev-feedback |
Focuses: | Cc: |
Description
WordPress shows IPv6 IPs for comments
- with only the last part of the IP
- without the : seperators.
- with all non-number parts of the IP stripped.
For example,
2001:0618:0400:f1a9:e000:dead:babe:cafe
is by WordPress shown as:
200161840019000
...which is very different from the IP. It's not even possible to make out what subnet the IP belongs to because the letters are stripped.
A little note on WordPress and IPv6: It works. It's almost "IPv6 ready". You can login. You can write posts. Everything which is "critical" works.
The ONLY issue with using WP on a IPv6-ready host that I've noticed is that it IPs all wrong.
Attachments (5)
Change History (34)
#2
@
16 years ago
WordPress 2.2 (yeah, I know, I should get around to upgrading) and WPMU 1.2.3. IPv6 IPs look the same in both WP branches, for example 2001618400192047617489 not 2001:0618:0400:f1a9:0204:76ff:fe17:489f (2001618400192047617489 could just as easily be 2001:f618:beef:babe:400f:1920:4761:7489 or.. a whole range of IPs, really)
#5
@
15 years ago
The culprit appears to be in changeset [3990]. I think the changes made to wp-includes/comment.php should just be reversed.
The data in $_SERVER['REMOTE_ADDR']
is filled in by the web server using information from the socket structure, so it seems to me there is little need to further "sanitize" it.
#8
@
15 years ago
- Owner changed from anonymous to pishmishy
- Status changed from new to assigned
We should just add a colon to the regular expression. I realize that it's improbable that we'd get bad data from the web server but it's better practice to mistrust this external data regardless.
#10
follow-up:
↓ 11
@
15 years ago
I think what ruckus is saying is that the regex should be
[:0-9a-zA-Z]
#11
in reply to:
↑ 10
;
follow-up:
↓ 12
@
15 years ago
Of course, sorry. Although you missed out the '.' :-) We also don't need characters after f.
[^0-9a-fA-F.:, ]
(Question: why does the original regex include a comma, space and ^
? I don't think those are necessary)
#12
in reply to:
↑ 11
@
15 years ago
Replying to pishmishy:
Of course, sorry. Although you missed out the '.' :-) We also don't need characters after f.
[^0-9a-fA-F.:, ]
(Question: why does the original regex include a comma, space and
^
? I don't think those are necessary)
The ^ is an invert on the replace - i.e. replace anything that doesn't match the regex
Comma and Space are needed because $_SERVER['REMOTE_ADDR']
can contain multiple IPs I believe.
#13
@
15 years ago
- Keywords needs-testing added
Revised patch attached. Needs testing though as I haven't got an IPv6 WordPress install.
#16
@
15 years ago
I don't think comma and space should be included, if we really want to have such strict checking. I don't see how there could be multiple IP addresses in $_SERVER['REMOTE_ADDR']
. If someone knows how this can happen, it should be documented. A network connection only has 2 end-points, local and remote.
However, I'd like to vote once more for less strict filtering of the data. We should protect against SQL injection, but not more. Having overly strict filtering doesn't have any benefits that I can see, but can cause unnecessary problems if new address formats come up in the future.
At the very minimum we should not mangle the value, but rather record something like the static string "invalid" if we don't like the contents. I don't think storing a mangled value (like is currently happening with IPv6) has any useful value.
I'd produce a new patch, but I couldn't find out a couple of things:
- where is the comment data escaped for database injection currently, to protect against SQL injection?
- where is
$postc
defined?
#17
follow-up:
↓ 19
@
15 years ago
It is too bad that these functions don't work with IPv6, because we could have just used them.
ip2long()
and long2ip()
#18
@
15 years ago
http://xref.redalt.com/wptrunk/_variables/index.htm#postc
According to this, $postc
is referenced 12 times, and all the references are in the get_commentdata()
function. Maybe it is deprecated already then.
The variable index doesn't list $postc
as being defined anywhere.
#19
in reply to:
↑ 17
@
15 years ago
Replying to santosj:
ip2long()
andlong2ip()
Theoretically, the remote address could be e.g. an AF_UNIX socket, which would be represented as a path name. Assuming it is only ever either IPv4 (AF_INET) or IPv6 (AF_INET6) would be wrong.
Not to mention ISO TP4 or SNA or DECnet or Apple Talk or IPX or ...
#21
follow-up:
↓ 22
@
15 years ago
- Keywords tested dev-feedback added; needs-testing removed
Patch 4579.patch seems to fix the problem for me on 2.3.2.
#22
in reply to:
↑ 21
@
15 years ago
- Owner changed from pishmishy to westi
- Status changed from assigned to new
#24
follow-up:
↓ 25
@
15 years ago
- Resolution fixed deleted
- Status changed from closed to reopened
[6658] only changes one of the two instances where filtering takes place.
I still think it would be sufficient to just escape the data before inserting in the database, no other filtering necessary.
If we do filtering, we should document why the different characters are allowed. I don't see how space and comma would ever be in REMOTE_ADDR.
#25
in reply to:
↑ 24
@
15 years ago
Replying to ruckus:
[6658] only changes one of the two instances where filtering takes place.
Why on earth that code is there twice I don't know. It makes no sense!
I still think it would be sufficient to just escape the data before inserting in the database, no other filtering necessary.
Not really as we want IPs rather than just safe data in that field.
If we do filtering, we should document why the different characters are allowed. I don't see how space and comma would ever be in REMOTE_ADDR.
As I have said above. AFAIK it is perfectly possible for REMOTE_ADDR to have a list of addresses in it - for example if the incoming request comes via a chain of proxy servers.
#27
@
15 years ago
When a request is proxied, the IP addresses are collected in the HTTP header X-Forwarded-For. That is a comma separated list.
The REMOTE_ADDR comes from the web server and records the address (IP or other protocol) of the remote end of the connection to the server. It is always a single address. Its definition comes from the CGI environment spec: http://hoohoo.ncsa.uiuc.edu/cgi/env.html
Can you show an example of web server software or other circumstances when REMOTE_ADDR has a comma separated list of addresses?
xiand0, guessing you are using WordPress 2.2.1?