Skip to content

Commit 31d60cd

Browse files
rolandshoemakerrolandshoemaker
committed
[release-branch.go1.15] net: verify results from Lookup* are valid domain names
For the methods LookupCNAME, LookupSRV, LookupMX, LookupNS, and LookupAddr check that the returned domain names are in fact valid DNS names using the existing isDomainName function. Thanks to Philipp Jeitner and Haya Shulman from Fraunhofer SIT for reporting this issue. Updates #46241 Fixes #46356 Fixes CVE-2021-33195 Change-Id: I47a4f58c031cb752f732e88bbdae7f819f0af4f3 Reviewed-on: https://go-review.googlesource.com/c/go/+/323131 Trust: Roland Shoemaker <[email protected]> Run-TryBot: Roland Shoemaker <[email protected]> TryBot-Result: Go Bot <[email protected]> Reviewed-by: Filippo Valsorda <[email protected]> Reviewed-by: Katie Hockman <[email protected]> (cherry picked from commit cdcd028) Reviewed-on: https://go-review.googlesource.com/c/go/+/323269
1 parent df9ce19 commit 31d60cd

File tree

2 files changed

+255
-13
lines changed

2 files changed

+255
-13
lines changed

‎src/net/dnsclient_unix_test.go

Lines changed: 157 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1799,3 +1799,160 @@ func TestPTRandNonPTR(t *testing.T) {
17991799
t.Errorf("names = %q; want %q", names, want)
18001800
}
18011801
}
1802+
1803+
func TestCVE202133195(t *testing.T) {
1804+
fake := fakeDNSServer{
1805+
rh: func(n, _ string, q dnsmessage.Message, _ time.Time) (dnsmessage.Message, error) {
1806+
r := dnsmessage.Message{
1807+
Header: dnsmessage.Header{
1808+
ID: q.Header.ID,
1809+
Response: true,
1810+
RCode: dnsmessage.RCodeSuccess,
1811+
RecursionAvailable: true,
1812+
},
1813+
Questions: q.Questions,
1814+
}
1815+
switch q.Questions[0].Type {
1816+
case dnsmessage.TypeCNAME:
1817+
r.Answers = []dnsmessage.Resource{}
1818+
case dnsmessage.TypeA: // CNAME lookup uses a A/AAAA as a proxy
1819+
r.Answers = append(r.Answers,
1820+
dnsmessage.Resource{
1821+
Header: dnsmessage.ResourceHeader{
1822+
Name: dnsmessage.MustNewName("<html>.golang.org."),
1823+
Type: dnsmessage.TypeA,
1824+
Class: dnsmessage.ClassINET,
1825+
Length: 4,
1826+
},
1827+
Body: &dnsmessage.AResource{
1828+
A: TestAddr,
1829+
},
1830+
},
1831+
)
1832+
case dnsmessage.TypeSRV:
1833+
n := q.Questions[0].Name
1834+
if n.String() == "_hdr._tcp.golang.org." {
1835+
n = dnsmessage.MustNewName("<html>.golang.org.")
1836+
}
1837+
r.Answers = append(r.Answers,
1838+
dnsmessage.Resource{
1839+
Header: dnsmessage.ResourceHeader{
1840+
Name: n,
1841+
Type: dnsmessage.TypeSRV,
1842+
Class: dnsmessage.ClassINET,
1843+
Length: 4,
1844+
},
1845+
Body: &dnsmessage.SRVResource{
1846+
Target: dnsmessage.MustNewName("<html>.golang.org."),
1847+
},
1848+
},
1849+
)
1850+
case dnsmessage.TypeMX:
1851+
r.Answers = append(r.Answers,
1852+
dnsmessage.Resource{
1853+
Header: dnsmessage.ResourceHeader{
1854+
Name: dnsmessage.MustNewName("<html>.golang.org."),
1855+
Type: dnsmessage.TypeMX,
1856+
Class: dnsmessage.ClassINET,
1857+
Length: 4,
1858+
},
1859+
Body: &dnsmessage.MXResource{
1860+
MX: dnsmessage.MustNewName("<html>.golang.org."),
1861+
},
1862+
},
1863+
)
1864+
case dnsmessage.TypeNS:
1865+
r.Answers = append(r.Answers,
1866+
dnsmessage.Resource{
1867+
Header: dnsmessage.ResourceHeader{
1868+
Name: dnsmessage.MustNewName("<html>.golang.org."),
1869+
Type: dnsmessage.TypeNS,
1870+
Class: dnsmessage.ClassINET,
1871+
Length: 4,
1872+
},
1873+
Body: &dnsmessage.NSResource{
1874+
NS: dnsmessage.MustNewName("<html>.golang.org."),
1875+
},
1876+
},
1877+
)
1878+
case dnsmessage.TypePTR:
1879+
r.Answers = append(r.Answers,
1880+
dnsmessage.Resource{
1881+
Header: dnsmessage.ResourceHeader{
1882+
Name: dnsmessage.MustNewName("<html>.golang.org."),
1883+
Type: dnsmessage.TypePTR,
1884+
Class: dnsmessage.ClassINET,
1885+
Length: 4,
1886+
},
1887+
Body: &dnsmessage.PTRResource{
1888+
PTR: dnsmessage.MustNewName("<html>.golang.org."),
1889+
},
1890+
},
1891+
)
1892+
}
1893+
return r, nil
1894+
},
1895+
}
1896+
1897+
r := Resolver{PreferGo: true, Dial: fake.DialContext}
1898+
// Change the default resolver to match our manipulated resolver
1899+
originalDefault := DefaultResolver
1900+
DefaultResolver = &r
1901+
defer func() {
1902+
DefaultResolver = originalDefault
1903+
}()
1904+
1905+
_, err := r.LookupCNAME(context.Background(), "golang.org")
1906+
if expected := "lookup golang.org: CNAME target is invalid"; err == nil || err.Error() != expected {
1907+
t.Errorf("Resolver.LookupCNAME returned unexpected error, got %q, want %q", err.Error(), expected)
1908+
}
1909+
_, err = LookupCNAME("golang.org")
1910+
if expected := "lookup golang.org: CNAME target is invalid"; err == nil || err.Error() != expected {
1911+
t.Errorf("LookupCNAME returned unexpected error, got %q, want %q", err.Error(), expected)
1912+
}
1913+
1914+
_, _, err = r.LookupSRV(context.Background(), "target", "tcp", "golang.org")
1915+
if expected := "lookup golang.org: SRV target is invalid"; err == nil || err.Error() != expected {
1916+
t.Errorf("Resolver.LookupSRV returned unexpected error, got %q, want %q", err.Error(), expected)
1917+
}
1918+
_, _, err = LookupSRV("target", "tcp", "golang.org")
1919+
if expected := "lookup golang.org: SRV target is invalid"; err == nil || err.Error() != expected {
1920+
t.Errorf("LookupSRV returned unexpected error, got %q, want %q", err.Error(), expected)
1921+
}
1922+
1923+
_, _, err = r.LookupSRV(context.Background(), "hdr", "tcp", "golang.org")
1924+
if expected := "lookup golang.org: SRV header name is invalid"; err == nil || err.Error() != expected {
1925+
t.Errorf("Resolver.LookupSRV returned unexpected error, got %q, want %q", err.Error(), expected)
1926+
}
1927+
_, _, err = LookupSRV("hdr", "tcp", "golang.org")
1928+
if expected := "lookup golang.org: SRV header name is invalid"; err == nil || err.Error() != expected {
1929+
t.Errorf("LookupSRV returned unexpected error, got %q, want %q", err.Error(), expected)
1930+
}
1931+
1932+
_, err = r.LookupMX(context.Background(), "golang.org")
1933+
if expected := "lookup golang.org: MX target is invalid"; err == nil || err.Error() != expected {
1934+
t.Errorf("Resolver.LookupMX returned unexpected error, got %q, want %q", err.Error(), expected)
1935+
}
1936+
_, err = LookupMX("golang.org")
1937+
if expected := "lookup golang.org: MX target is invalid"; err == nil || err.Error() != expected {
1938+
t.Errorf("LookupMX returned unexpected error, got %q, want %q", err.Error(), expected)
1939+
}
1940+
1941+
_, err = r.LookupNS(context.Background(), "golang.org")
1942+
if expected := "lookup golang.org: NS target is invalid"; err == nil || err.Error() != expected {
1943+
t.Errorf("Resolver.LookupNS returned unexpected error, got %q, want %q", err.Error(), expected)
1944+
}
1945+
_, err = LookupNS("golang.org")
1946+
if expected := "lookup golang.org: NS target is invalid"; err == nil || err.Error() != expected {
1947+
t.Errorf("LookupNS returned unexpected error, got %q, want %q", err.Error(), expected)
1948+
}
1949+
1950+
_, err = r.LookupAddr(context.Background(), "1.2.3.4")
1951+
if expected := "lookup 1.2.3.4: PTR target is invalid"; err == nil || err.Error() != expected {
1952+
t.Errorf("Resolver.LookupAddr returned unexpected error, got %q, want %q", err.Error(), expected)
1953+
}
1954+
_, err = LookupAddr("1.2.3.4")
1955+
if expected := "lookup 1.2.3.4: PTR target is invalid"; err == nil || err.Error() != expected {
1956+
t.Errorf("LookupAddr returned unexpected error, got %q, want %q", err.Error(), expected)
1957+
}
1958+
}

‎src/net/lookup.go

Lines changed: 98 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -389,8 +389,11 @@ func (r *Resolver) LookupPort(ctx context.Context, network, service string) (por
389389
// LookupCNAME does not return an error if host does not
390390
// contain DNS "CNAME" records, as long as host resolves to
391391
// address records.
392+
//
393+
// The returned canonical name is validated to be a properly
394+
// formatted presentation-format domain name.
392395
func LookupCNAME(host string) (cname string, err error) {
393-
return DefaultResolver.lookupCNAME(context.Background(), host)
396+
return DefaultResolver.LookupCNAME(context.Background(), host)
394397
}
395398

396399
// LookupCNAME returns the canonical name for the given host.
@@ -403,8 +406,18 @@ func LookupCNAME(host string) (cname string, err error) {
403406
// LookupCNAME does not return an error if host does not
404407
// contain DNS "CNAME" records, as long as host resolves to
405408
// address records.
406-
func (r *Resolver) LookupCNAME(ctx context.Context, host string) (cname string, err error) {
407-
return r.lookupCNAME(ctx, host)
409+
//
410+
// The returned canonical name is validated to be a properly
411+
// formatted presentation-format domain name.
412+
func (r *Resolver) LookupCNAME(ctx context.Context, host string) (string, error) {
413+
cname, err := r.lookupCNAME(ctx, host)
414+
if err != nil {
415+
return "", err
416+
}
417+
if !isDomainName(cname) {
418+
return "", &DNSError{Err: "CNAME target is invalid", Name: host}
419+
}
420+
return cname, nil
408421
}
409422

410423
// LookupSRV tries to resolve an SRV query of the given service,
@@ -416,8 +429,11 @@ func (r *Resolver) LookupCNAME(ctx context.Context, host string) (cname string,
416429
// That is, it looks up _service._proto.name. To accommodate services
417430
// publishing SRV records under non-standard names, if both service
418431
// and proto are empty strings, LookupSRV looks up name directly.
432+
//
433+
// The returned service names are validated to be properly
434+
// formatted presentation-format domain names.
419435
func LookupSRV(service, proto, name string) (cname string, addrs []*SRV, err error) {
420-
return DefaultResolver.lookupSRV(context.Background(), service, proto, name)
436+
return DefaultResolver.LookupSRV(context.Background(), service, proto, name)
421437
}
422438

423439
// LookupSRV tries to resolve an SRV query of the given service,
@@ -429,28 +445,82 @@ func LookupSRV(service, proto, name string) (cname string, addrs []*SRV, err err
429445
// That is, it looks up _service._proto.name. To accommodate services
430446
// publishing SRV records under non-standard names, if both service
431447
// and proto are empty strings, LookupSRV looks up name directly.
432-
func (r *Resolver) LookupSRV(ctx context.Context, service, proto, name string) (cname string, addrs []*SRV, err error) {
433-
return r.lookupSRV(ctx, service, proto, name)
448+
//
449+
// The returned service names are validated to be properly
450+
// formatted presentation-format domain names.
451+
func (r *Resolver) LookupSRV(ctx context.Context, service, proto, name string) (string, []*SRV, error) {
452+
cname, addrs, err := r.lookupSRV(ctx, service, proto, name)
453+
if err != nil {
454+
return "", nil, err
455+
}
456+
if cname != "" && !isDomainName(cname) {
457+
return "", nil, &DNSError{Err: "SRV header name is invalid", Name: name}
458+
}
459+
for _, addr := range addrs {
460+
if addr == nil {
461+
continue
462+
}
463+
if !isDomainName(addr.Target) {
464+
return "", nil, &DNSError{Err: "SRV target is invalid", Name: name}
465+
}
466+
}
467+
return cname, addrs, nil
434468
}
435469

436470
// LookupMX returns the DNS MX records for the given domain name sorted by preference.
471+
//
472+
// The returned mail server names are validated to be properly
473+
// formatted presentation-format domain names.
437474
func LookupMX(name string) ([]*MX, error) {
438-
return DefaultResolver.lookupMX(context.Background(), name)
475+
return DefaultResolver.LookupMX(context.Background(), name)
439476
}
440477

441478
// LookupMX returns the DNS MX records for the given domain name sorted by preference.
479+
//
480+
// The returned mail server names are validated to be properly
481+
// formatted presentation-format domain names.
442482
func (r *Resolver) LookupMX(ctx context.Context, name string) ([]*MX, error) {
443-
return r.lookupMX(ctx, name)
483+
records, err := r.lookupMX(ctx, name)
484+
if err != nil {
485+
return nil, err
486+
}
487+
for _, mx := range records {
488+
if mx == nil {
489+
continue
490+
}
491+
if !isDomainName(mx.Host) {
492+
return nil, &DNSError{Err: "MX target is invalid", Name: name}
493+
}
494+
}
495+
return records, nil
444496
}
445497

446498
// LookupNS returns the DNS NS records for the given domain name.
499+
//
500+
// The returned name server names are validated to be properly
501+
// formatted presentation-format domain names.
447502
func LookupNS(name string) ([]*NS, error) {
448-
return DefaultResolver.lookupNS(context.Background(), name)
503+
return DefaultResolver.LookupNS(context.Background(), name)
449504
}
450505

451506
// LookupNS returns the DNS NS records for the given domain name.
507+
//
508+
// The returned name server names are validated to be properly
509+
// formatted presentation-format domain names.
452510
func (r *Resolver) LookupNS(ctx context.Context, name string) ([]*NS, error) {
453-
return r.lookupNS(ctx, name)
511+
records, err := r.lookupNS(ctx, name)
512+
if err != nil {
513+
return nil, err
514+
}
515+
for _, ns := range records {
516+
if ns == nil {
517+
continue
518+
}
519+
if !isDomainName(ns.Host) {
520+
return nil, &DNSError{Err: "NS target is invalid", Name: name}
521+
}
522+
}
523+
return records, nil
454524
}
455525

456526
// LookupTXT returns the DNS TXT records for the given domain name.
@@ -466,14 +536,29 @@ func (r *Resolver) LookupTXT(ctx context.Context, name string) ([]string, error)
466536
// LookupAddr performs a reverse lookup for the given address, returning a list
467537
// of names mapping to that address.
468538
//
539+
// The returned names are validated to be properly formatted presentation-format
540+
// domain names.
541+
//
469542
// When using the host C library resolver, at most one result will be
470543
// returned. To bypass the host resolver, use a custom Resolver.
471544
func LookupAddr(addr string) (names []string, err error) {
472-
return DefaultResolver.lookupAddr(context.Background(), addr)
545+
return DefaultResolver.LookupAddr(context.Background(), addr)
473546
}
474547

475548
// LookupAddr performs a reverse lookup for the given address, returning a list
476549
// of names mapping to that address.
477-
func (r *Resolver) LookupAddr(ctx context.Context, addr string) (names []string, err error) {
478-
return r.lookupAddr(ctx, addr)
550+
//
551+
// The returned names are validated to be properly formatted presentation-format
552+
// domain names.
553+
func (r *Resolver) LookupAddr(ctx context.Context, addr string) ([]string, error) {
554+
names, err := r.lookupAddr(ctx, addr)
555+
if err != nil {
556+
return nil, err
557+
}
558+
for _, name := range names {
559+
if !isDomainName(name) {
560+
return nil, &DNSError{Err: "PTR target is invalid", Name: addr}
561+
}
562+
}
563+
return names, nil
479564
}

0 commit comments

Comments
 (0)