Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

net: reject Lookup("www.google.com..") on all systems #7122

Closed
vdobler opened this issue Jan 14, 2014 · 11 comments
Closed

net: reject Lookup("www.google.com..") on all systems #7122

vdobler opened this issue Jan 14, 2014 · 11 comments
Labels
FrozenDueToAge Suggested Issues that may be good for new contributors looking for work to do.
Milestone

Comments

@vdobler
Copy link
Contributor

vdobler commented Jan 14, 2014

On Linux domain look-up is too liberal and accepts domain
names with two trailing dots like "www.google.com..".
Most probably this is an issue on the OS level as
gethostbyname(3) shows this behavior also.

Allowing two trailing dots instead of none or
one (for a FQDN) seems harmless but it messes with the
domain name canonicalization done in net/http/cookiejar
and the determination of the public suffix of a domain.

I'd like to argue that malformed domains should be
rejected on look-up on any OS; at least no request
should be made to such malformed domain names.

This issue is not present on Windows and OSX.
Domains with three trailing dots are properly rejected
on Linux, Windows and OSX.
Other OS are untested.

The following program demonstrates the issue:
On Linux neither net.LookupHost nor http.Get fail.

--------------
package main

import (
    "io"
    "log"
    "net"
    "net/http"
    "os"
)

func main() {
    addrs, err := net.LookupHost("www.google.com..")
    if err != nil {
        log.Fatal(err)
    }
    for _, a := range addrs {
        log.Println(a)
    }
    res, err := http.Get("http://www.google.com../foo")
    if err != nil {
        log.Fatal(err)
    }
    defer res.Body.Close()
    io.Copy(os.Stdout, res.Body)
}
@mikioh
Copy link
Contributor

mikioh commented Jan 15, 2014

Comment 1:

Perhaps we should ask rsc (or iant?) why did you guys choose hostname-like
platform-dependent (but plain and  flexible) namespace instead of host subcomponent of
URI namespace. Anyway, the current net package takes both namespaces and never
recognises the difference btw those twos. I personally hope that packages that have to
take account of their property's namespace would handle this issue at each package.

@nigeltao
Copy link
Contributor

Comment 2:

This is possibly a glibc bug. In any case, I filed
https://sourceware.org/bugzilla/show_bug.cgi?id=16469

@rsc
Copy link
Contributor

rsc commented Jan 22, 2014

Comment 3:

I do not understand the question in #1.
I agree that net.LookupHost("www.google.com..") should be made to fail everywhere.

@mikioh
Copy link
Contributor

mikioh commented Jan 22, 2014

Comment 4:

I'm not good at this section but what if we have some entries in local database (e.g.
/etc/hosts) like the following:
::1  www.google.com..
127.0.0.1  www.google.com..
getaddrinfo (or similar kernel/system library service) on some implementation works as
not only a DNS resolver but mDNS or some network instance (that is represented in
net-unicode, see http://tools.ietf.org/html/rfc6762#appendix-F) resolver.
# ping6 www.google.com..
PING6(56=40+8+8 bytes) ::1 --> ::1
16 bytes from ::1, icmp_seq=0 hlim=64 time=0.092 ms
# ping www.google.com..
PING www.google.com.. (127.0.0.1): 56 data bytes
64 bytes from 127.0.0.1: icmp_seq=0 ttl=64 time=0.059 ms
I have no concrete opinion about this issue, whether we should normalize network
instance/node namespace in the package net API level.

@rsc
Copy link
Contributor

rsc commented Mar 3, 2014

Comment 5:

Labels changed: added release-go1.3.

Status changed to Accepted.

@rsc
Copy link
Contributor

rsc commented May 9, 2014

Comment 6:

Labels changed: added release-none, suggested, removed release-go1.3.

@griesemer
Copy link
Contributor

Comment 7:

Labels changed: added repo-main.

@vdobler vdobler added accepted Suggested Issues that may be good for new contributors looking for work to do. labels Oct 1, 2014
@rsc rsc added this to the Unplanned milestone Apr 10, 2015
@gopherbot
Copy link

CL https://golang.org/cl/37843 mentions this issue.

gopherbot pushed a commit that referenced this issue Mar 21, 2017
The old implementation of Jar made the assumption that the host names
in the URLs given to SetCookies() and Cookies() methods are well-formed.
This is not an unreasonable assumption as malformed host names do not
trigger calls to SetCookies or Cookies (at least not from net/http)
as the HTTP request themselves are not executed. But there can be other
invocations of these methods and at least on Linux it was possible to
make DNS lookup to domain names with two trailing dots (see issue #7122).

This is an old bug and this CL revives an old change (see
https://codereview.appspot.com/52100043) to fix the issue. The discussion
around 52100043 focused on the interplay between the jar and the
public suffix list and who is responsible for which type if domain name
canonicalization. The new bug report in issue #19384 used a nil public
suffix list which demonstrates that the package cookiejar alone exhibits
this problem and any solution cannot be fully delegated to the
implementation of the used PublicSuffixList: Package cookiejar itself
needs to protect against host names of the form ".." which triggered an
out-of-bounds error.

This CL does not address the issue of host name canonicalization and
the question who is responsible for it. This CL just prevents the
out-of-bounds error: It is a very conservative change, i.e. one might
still set and retrieve cookies for host names like "weird.stuf...".
Several more test cases document how the current code works.

Fixes #19384.

Change-Id: I14be080e8a2a0b266ced779f2aeb18841b730610
Reviewed-on: https://go-review.googlesource.com/37843
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
@odeke-em
Copy link
Member

odeke-em commented Jun 7, 2020

Thank you for filing this bug @vdobler! This issue seems to have been filed when we were on code.google.com so I shall raise some attention to it. Kindly paging @iangudger, you might be interested in this bug, if you are free and bored.

@r5d
Copy link
Contributor

r5d commented Sep 11, 2021

@odeke-em, net.LookupHost("www.google.com..") is currently failing on Linux:

$ uname -a
Linux cygnus 5.10.0-8-amd64 #1 SMP Debian 5.10.46-4 (2021-08-03) x86_64 GNU/Linux
$ go version
go version go1.17.1 linux/amd64
$ ./7122
2021/09/11 18:04:13 lookup www.google.com..: no such host

Perhaps, this can be closed now?

@ianlancetaylor
Copy link
Contributor

Sounds like it. Thanks.

@golang golang locked and limited conversation to collaborators Sep 12, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge Suggested Issues that may be good for new contributors looking for work to do.
Projects
None yet
Development

No branches or pull requests

9 participants