Skip to content

Commit

Permalink
dns/dnsmessage: reject compressed SRV resource records
Browse files Browse the repository at this point in the history
Updates golang/go#10622

Change-Id: Iadf0ff0fd223a315130941464040aef5e71f6130
Reviewed-on: https://go-review.googlesource.com/100055
Run-TryBot: Brad Fitzpatrick <[email protected]>
TryBot-Result: Gobot Gobot <[email protected]>
Reviewed-by: Matthew Dempsky <[email protected]>
  • Loading branch information
iangudger committed Mar 17, 2018
1 parent e0c57d8 commit 24dd378
Show file tree
Hide file tree
Showing 2 changed files with 31 additions and 1 deletion.
10 changes: 9 additions & 1 deletion dns/dnsmessage/message.go
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,7 @@ var (
errTooManyAdditionals = errors.New("too many Additionals to pack (>65535)")
errNonCanonicalName = errors.New("name is not in canonical format (it must end with a .)")
errStringTooLong = errors.New("character string exceeds maximum length (255)")
errCompressedSRV = errors.New("compressed name in SRV resource data")
)

// Internal constants.
Expand Down Expand Up @@ -1610,6 +1611,10 @@ func (n *Name) pack(msg []byte, compression map[string]int, compressionOff int)

// unpack unpacks a domain name.
func (n *Name) unpack(msg []byte, off int) (int, error) {
return n.unpackCompressed(msg, off, true /* allowCompression */)
}

func (n *Name) unpackCompressed(msg []byte, off int, allowCompression bool) (int, error) {
// currOff is the current working offset.
currOff := off

Expand Down Expand Up @@ -1645,6 +1650,9 @@ Loop:
name = append(name, '.')
currOff = endOff
case 0xC0: // Pointer
if !allowCompression {
return off, errCompressedSRV
}
if currOff >= len(msg) {
return off, errInvalidPtr
}
Expand Down Expand Up @@ -2044,7 +2052,7 @@ func unpackSRVResource(msg []byte, off int) (SRVResource, error) {
return SRVResource{}, &nestedError{"Port", err}
}
var target Name
if _, err := target.unpack(msg, off); err != nil {
if _, err := target.unpackCompressed(msg, off, false /* allowCompression */); err != nil {

This comment has been minimized.

Copy link
@strk

strk Sep 24, 2019

isn't this against the robustness principle ? Be tolerant in what you accept !

This comment has been minimized.

Copy link
@ianlancetaylor

ianlancetaylor Sep 25, 2019

Member

Note that we do not use GitHub for code review, so very few people will see this comment. If you want more people to see it, comment in Gerritt at https://golang.org/cl/100055. Thanks.

return SRVResource{}, &nestedError{"Target", err}
}
return SRVResource{priority, weight, port, target}, nil
Expand Down
22 changes: 22 additions & 0 deletions dns/dnsmessage/message_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,28 @@ func TestNamePackUnpack(t *testing.T) {
}
}

func TestIncompressibleName(t *testing.T) {
name := mustNewName("example.com.")
compression := map[string]int{}
buf, err := name.pack(make([]byte, 0, 100), compression, 0)
if err != nil {
t.Fatal("First packing failed:", err)
}
buf, err = name.pack(buf, compression, 0)
if err != nil {
t.Fatal("Second packing failed:", err)
}
var n1 Name
off, err := n1.unpackCompressed(buf, 0, false /* allowCompression */)
if err != nil {
t.Fatal("Unpacking incompressible name without pointers failed:", err)
}
var n2 Name
if _, err := n2.unpackCompressed(buf, off, false /* allowCompression */); err != errCompressedSRV {
t.Errorf("Unpacking compressed incompressible name with pointers: got err = %v, want = %v", err, errCompressedSRV)
}
}

func checkErrorPrefix(err error, prefix string) bool {
e, ok := err.(*nestedError)
return ok && e.s == prefix
Expand Down

0 comments on commit 24dd378

Please sign in to comment.