Skip to content

Commit cdcd028

Browse files
rolandshoemakerrolandshoemaker
committed
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. Fixes #46241 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]>
1 parent 8bf5bf5 commit cdcd028

2 files changed

Lines changed: 255 additions & 13 deletions

File tree

‎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
@@ -396,10 +396,13 @@ func (r *Resolver) LookupPort(ctx context.Context, network, service string) (por
396396
// contain DNS "CNAME" records, as long as host resolves to
397397
// address records.
398398
//
399+
// The returned canonical name is validated to be a properly
400+
// formatted presentation-format domain name.
401+
//
399402
// LookupCNAME uses context.Background internally; to specify the context, use
400403
// Resolver.LookupCNAME.
401404
func LookupCNAME(host string) (cname string, err error) {
402-
return DefaultResolver.lookupCNAME(context.Background(), host)
405+
return DefaultResolver.LookupCNAME(context.Background(), host)
403406
}
404407

405408
// LookupCNAME returns the canonical name for the given host.
@@ -412,8 +415,18 @@ func LookupCNAME(host string) (cname string, err error) {
412415
// LookupCNAME does not return an error if host does not
413416
// contain DNS "CNAME" records, as long as host resolves to
414417
// address records.
415-
func (r *Resolver) LookupCNAME(ctx context.Context, host string) (cname string, err error) {
416-
return r.lookupCNAME(ctx, host)
418+
//
419+
// The returned canonical name is validated to be a properly
420+
// formatted presentation-format domain name.
421+
func (r *Resolver) LookupCNAME(ctx context.Context, host string) (string, error) {
422+
cname, err := r.lookupCNAME(ctx, host)
423+
if err != nil {
424+
return "", err
425+
}
426+
if !isDomainName(cname) {
427+
return "", &DNSError{Err: "CNAME target is invalid", Name: host}
428+
}
429+
return cname, nil
417430
}
418431

419432
// LookupSRV tries to resolve an SRV query of the given service,
@@ -425,8 +438,11 @@ func (r *Resolver) LookupCNAME(ctx context.Context, host string) (cname string,
425438
// That is, it looks up _service._proto.name. To accommodate services
426439
// publishing SRV records under non-standard names, if both service
427440
// and proto are empty strings, LookupSRV looks up name directly.
441+
//
442+
// The returned service names are validated to be properly
443+
// formatted presentation-format domain names.
428444
func LookupSRV(service, proto, name string) (cname string, addrs []*SRV, err error) {
429-
return DefaultResolver.lookupSRV(context.Background(), service, proto, name)
445+
return DefaultResolver.LookupSRV(context.Background(), service, proto, name)
430446
}
431447

432448
// LookupSRV tries to resolve an SRV query of the given service,
@@ -438,34 +454,88 @@ func LookupSRV(service, proto, name string) (cname string, addrs []*SRV, err err
438454
// That is, it looks up _service._proto.name. To accommodate services
439455
// publishing SRV records under non-standard names, if both service
440456
// and proto are empty strings, LookupSRV looks up name directly.
441-
func (r *Resolver) LookupSRV(ctx context.Context, service, proto, name string) (cname string, addrs []*SRV, err error) {
442-
return r.lookupSRV(ctx, service, proto, name)
457+
//
458+
// The returned service names are validated to be properly
459+
// formatted presentation-format domain names.
460+
func (r *Resolver) LookupSRV(ctx context.Context, service, proto, name string) (string, []*SRV, error) {
461+
cname, addrs, err := r.lookupSRV(ctx, service, proto, name)
462+
if err != nil {
463+
return "", nil, err
464+
}
465+
if cname != "" && !isDomainName(cname) {
466+
return "", nil, &DNSError{Err: "SRV header name is invalid", Name: name}
467+
}
468+
for _, addr := range addrs {
469+
if addr == nil {
470+
continue
471+
}
472+
if !isDomainName(addr.Target) {
473+
return "", nil, &DNSError{Err: "SRV target is invalid", Name: name}
474+
}
475+
}
476+
return cname, addrs, nil
443477
}
444478

445479
// LookupMX returns the DNS MX records for the given domain name sorted by preference.
446480
//
481+
// The returned mail server names are validated to be properly
482+
// formatted presentation-format domain names.
483+
//
447484
// LookupMX uses context.Background internally; to specify the context, use
448485
// Resolver.LookupMX.
449486
func LookupMX(name string) ([]*MX, error) {
450-
return DefaultResolver.lookupMX(context.Background(), name)
487+
return DefaultResolver.LookupMX(context.Background(), name)
451488
}
452489

453490
// LookupMX returns the DNS MX records for the given domain name sorted by preference.
491+
//
492+
// The returned mail server names are validated to be properly
493+
// formatted presentation-format domain names.
454494
func (r *Resolver) LookupMX(ctx context.Context, name string) ([]*MX, error) {
455-
return r.lookupMX(ctx, name)
495+
records, err := r.lookupMX(ctx, name)
496+
if err != nil {
497+
return nil, err
498+
}
499+
for _, mx := range records {
500+
if mx == nil {
501+
continue
502+
}
503+
if !isDomainName(mx.Host) {
504+
return nil, &DNSError{Err: "MX target is invalid", Name: name}
505+
}
506+
}
507+
return records, nil
456508
}
457509

458510
// LookupNS returns the DNS NS records for the given domain name.
459511
//
512+
// The returned name server names are validated to be properly
513+
// formatted presentation-format domain names.
514+
//
460515
// LookupNS uses context.Background internally; to specify the context, use
461516
// Resolver.LookupNS.
462517
func LookupNS(name string) ([]*NS, error) {
463-
return DefaultResolver.lookupNS(context.Background(), name)
518+
return DefaultResolver.LookupNS(context.Background(), name)
464519
}
465520

466521
// LookupNS returns the DNS NS records for the given domain name.
522+
//
523+
// The returned name server names are validated to be properly
524+
// formatted presentation-format domain names.
467525
func (r *Resolver) LookupNS(ctx context.Context, name string) ([]*NS, error) {
468-
return r.lookupNS(ctx, name)
526+
records, err := r.lookupNS(ctx, name)
527+
if err != nil {
528+
return nil, err
529+
}
530+
for _, ns := range records {
531+
if ns == nil {
532+
continue
533+
}
534+
if !isDomainName(ns.Host) {
535+
return nil, &DNSError{Err: "NS target is invalid", Name: name}
536+
}
537+
}
538+
return records, nil
469539
}
470540

471541
// LookupTXT returns the DNS TXT records for the given domain name.
@@ -484,17 +554,32 @@ func (r *Resolver) LookupTXT(ctx context.Context, name string) ([]string, error)
484554
// LookupAddr performs a reverse lookup for the given address, returning a list
485555
// of names mapping to that address.
486556
//
557+
// The returned names are validated to be properly formatted presentation-format
558+
// domain names.
559+
//
487560
// When using the host C library resolver, at most one result will be
488561
// returned. To bypass the host resolver, use a custom Resolver.
489562
//
490563
// LookupAddr uses context.Background internally; to specify the context, use
491564
// Resolver.LookupAddr.
492565
func LookupAddr(addr string) (names []string, err error) {
493-
return DefaultResolver.lookupAddr(context.Background(), addr)
566+
return DefaultResolver.LookupAddr(context.Background(), addr)
494567
}
495568

496569
// LookupAddr performs a reverse lookup for the given address, returning a list
497570
// of names mapping to that address.
498-
func (r *Resolver) LookupAddr(ctx context.Context, addr string) (names []string, err error) {
499-
return r.lookupAddr(ctx, addr)
571+
//
572+
// The returned names are validated to be properly formatted presentation-format
573+
// domain names.
574+
func (r *Resolver) LookupAddr(ctx context.Context, addr string) ([]string, error) {
575+
names, err := r.lookupAddr(ctx, addr)
576+
if err != nil {
577+
return nil, err
578+
}
579+
for _, name := range names {
580+
if !isDomainName(name) {
581+
return nil, &DNSError{Err: "PTR target is invalid", Name: addr}
582+
}
583+
}
584+
return names, nil
500585
}

0 commit comments

Comments
 (0)