dns/dnsmessage: fix bug in length fixup
If during packing, the byte slice gets re-allocated between packing the
ResourceHeader and ResourceBody, the length will get updated in the old
byte slice before re-allocation resulting in a zero length in the final
packed message.
This change fixes the bug by passing the offset at which the length
should be written instead of a slice of the output byte slice from
ResourceHeader encoding step.
Updates golang/go#16218
Change-Id: Ifd7e2f549b7087ed5b52eaa6ae78970fec4ad988
Reviewed-on: https://go-review.googlesource.com/123835
Run-TryBot: Ian Gudger <igudger@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
diff --git a/dns/dnsmessage/message.go b/dns/dnsmessage/message.go
index dc5b143..13fbc08 100644
--- a/dns/dnsmessage/message.go
+++ b/dns/dnsmessage/message.go
@@ -493,7 +493,7 @@
}
oldMsg := msg
r.Header.Type = r.Body.realType()
- msg, length, err := r.Header.pack(msg, compression, compressionOff)
+ msg, lenOff, err := r.Header.pack(msg, compression, compressionOff)
if err != nil {
return msg, &nestedError{"ResourceHeader", err}
}
@@ -502,7 +502,7 @@
if err != nil {
return msg, &nestedError{"content", err}
}
- if err := r.Header.fixLen(msg, length, preLen); err != nil {
+ if err := r.Header.fixLen(msg, lenOff, preLen); err != nil {
return oldMsg, err
}
return msg, nil
@@ -1323,7 +1323,7 @@
return err
}
h.Type = r.realType()
- msg, length, err := h.pack(b.msg, b.compression, b.start)
+ msg, lenOff, err := h.pack(b.msg, b.compression, b.start)
if err != nil {
return &nestedError{"ResourceHeader", err}
}
@@ -1331,7 +1331,7 @@
if msg, err = r.pack(msg, b.compression, b.start); err != nil {
return &nestedError{"CNAMEResource body", err}
}
- if err := h.fixLen(msg, length, preLen); err != nil {
+ if err := h.fixLen(msg, lenOff, preLen); err != nil {
return err
}
if err := b.incrementSectionCount(); err != nil {
@@ -1347,7 +1347,7 @@
return err
}
h.Type = r.realType()
- msg, length, err := h.pack(b.msg, b.compression, b.start)
+ msg, lenOff, err := h.pack(b.msg, b.compression, b.start)
if err != nil {
return &nestedError{"ResourceHeader", err}
}
@@ -1355,7 +1355,7 @@
if msg, err = r.pack(msg, b.compression, b.start); err != nil {
return &nestedError{"MXResource body", err}
}
- if err := h.fixLen(msg, length, preLen); err != nil {
+ if err := h.fixLen(msg, lenOff, preLen); err != nil {
return err
}
if err := b.incrementSectionCount(); err != nil {
@@ -1371,7 +1371,7 @@
return err
}
h.Type = r.realType()
- msg, length, err := h.pack(b.msg, b.compression, b.start)
+ msg, lenOff, err := h.pack(b.msg, b.compression, b.start)
if err != nil {
return &nestedError{"ResourceHeader", err}
}
@@ -1379,7 +1379,7 @@
if msg, err = r.pack(msg, b.compression, b.start); err != nil {
return &nestedError{"NSResource body", err}
}
- if err := h.fixLen(msg, length, preLen); err != nil {
+ if err := h.fixLen(msg, lenOff, preLen); err != nil {
return err
}
if err := b.incrementSectionCount(); err != nil {
@@ -1395,7 +1395,7 @@
return err
}
h.Type = r.realType()
- msg, length, err := h.pack(b.msg, b.compression, b.start)
+ msg, lenOff, err := h.pack(b.msg, b.compression, b.start)
if err != nil {
return &nestedError{"ResourceHeader", err}
}
@@ -1403,7 +1403,7 @@
if msg, err = r.pack(msg, b.compression, b.start); err != nil {
return &nestedError{"PTRResource body", err}
}
- if err := h.fixLen(msg, length, preLen); err != nil {
+ if err := h.fixLen(msg, lenOff, preLen); err != nil {
return err
}
if err := b.incrementSectionCount(); err != nil {
@@ -1419,7 +1419,7 @@
return err
}
h.Type = r.realType()
- msg, length, err := h.pack(b.msg, b.compression, b.start)
+ msg, lenOff, err := h.pack(b.msg, b.compression, b.start)
if err != nil {
return &nestedError{"ResourceHeader", err}
}
@@ -1427,7 +1427,7 @@
if msg, err = r.pack(msg, b.compression, b.start); err != nil {
return &nestedError{"SOAResource body", err}
}
- if err := h.fixLen(msg, length, preLen); err != nil {
+ if err := h.fixLen(msg, lenOff, preLen); err != nil {
return err
}
if err := b.incrementSectionCount(); err != nil {
@@ -1443,7 +1443,7 @@
return err
}
h.Type = r.realType()
- msg, length, err := h.pack(b.msg, b.compression, b.start)
+ msg, lenOff, err := h.pack(b.msg, b.compression, b.start)
if err != nil {
return &nestedError{"ResourceHeader", err}
}
@@ -1451,7 +1451,7 @@
if msg, err = r.pack(msg, b.compression, b.start); err != nil {
return &nestedError{"TXTResource body", err}
}
- if err := h.fixLen(msg, length, preLen); err != nil {
+ if err := h.fixLen(msg, lenOff, preLen); err != nil {
return err
}
if err := b.incrementSectionCount(); err != nil {
@@ -1467,7 +1467,7 @@
return err
}
h.Type = r.realType()
- msg, length, err := h.pack(b.msg, b.compression, b.start)
+ msg, lenOff, err := h.pack(b.msg, b.compression, b.start)
if err != nil {
return &nestedError{"ResourceHeader", err}
}
@@ -1475,7 +1475,7 @@
if msg, err = r.pack(msg, b.compression, b.start); err != nil {
return &nestedError{"SRVResource body", err}
}
- if err := h.fixLen(msg, length, preLen); err != nil {
+ if err := h.fixLen(msg, lenOff, preLen); err != nil {
return err
}
if err := b.incrementSectionCount(); err != nil {
@@ -1491,7 +1491,7 @@
return err
}
h.Type = r.realType()
- msg, length, err := h.pack(b.msg, b.compression, b.start)
+ msg, lenOff, err := h.pack(b.msg, b.compression, b.start)
if err != nil {
return &nestedError{"ResourceHeader", err}
}
@@ -1499,7 +1499,7 @@
if msg, err = r.pack(msg, b.compression, b.start); err != nil {
return &nestedError{"AResource body", err}
}
- if err := h.fixLen(msg, length, preLen); err != nil {
+ if err := h.fixLen(msg, lenOff, preLen); err != nil {
return err
}
if err := b.incrementSectionCount(); err != nil {
@@ -1515,7 +1515,7 @@
return err
}
h.Type = r.realType()
- msg, length, err := h.pack(b.msg, b.compression, b.start)
+ msg, lenOff, err := h.pack(b.msg, b.compression, b.start)
if err != nil {
return &nestedError{"ResourceHeader", err}
}
@@ -1523,7 +1523,7 @@
if msg, err = r.pack(msg, b.compression, b.start); err != nil {
return &nestedError{"AAAAResource body", err}
}
- if err := h.fixLen(msg, length, preLen); err != nil {
+ if err := h.fixLen(msg, lenOff, preLen); err != nil {
return err
}
if err := b.incrementSectionCount(); err != nil {
@@ -1539,7 +1539,7 @@
return err
}
h.Type = r.realType()
- msg, length, err := h.pack(b.msg, b.compression, b.start)
+ msg, lenOff, err := h.pack(b.msg, b.compression, b.start)
if err != nil {
return &nestedError{"ResourceHeader", err}
}
@@ -1547,7 +1547,7 @@
if msg, err = r.pack(msg, b.compression, b.start); err != nil {
return &nestedError{"OPTResource body", err}
}
- if err := h.fixLen(msg, length, preLen); err != nil {
+ if err := h.fixLen(msg, lenOff, preLen); err != nil {
return err
}
if err := b.incrementSectionCount(); err != nil {
@@ -1606,19 +1606,18 @@
// pack appends the wire format of the ResourceHeader to oldMsg.
//
-// The bytes where length was packed are returned as a slice so they can be
-// updated after the rest of the Resource has been packed.
-func (h *ResourceHeader) pack(oldMsg []byte, compression map[string]int, compressionOff int) (msg []byte, length []byte, err error) {
+// lenOff is the offset in msg where the Length field was packed.
+func (h *ResourceHeader) pack(oldMsg []byte, compression map[string]int, compressionOff int) (msg []byte, lenOff int, err error) {
msg = oldMsg
if msg, err = h.Name.pack(msg, compression, compressionOff); err != nil {
- return oldMsg, nil, &nestedError{"Name", err}
+ return oldMsg, 0, &nestedError{"Name", err}
}
msg = packType(msg, h.Type)
msg = packClass(msg, h.Class)
msg = packUint32(msg, h.TTL)
- lenBegin := len(msg)
+ lenOff = len(msg)
msg = packUint16(msg, h.Length)
- return msg, msg[lenBegin : lenBegin+uint16Len], nil
+ return msg, lenOff, nil
}
func (h *ResourceHeader) unpack(msg []byte, off int) (int, error) {
@@ -1642,14 +1641,20 @@
return newOff, nil
}
-func (h *ResourceHeader) fixLen(msg []byte, length []byte, preLen int) error {
+// fixLen updates a packed ResourceHeader to include the length of the
+// ResourceBody.
+//
+// lenOff is the offset of the ResourceHeader.Length field in msg.
+//
+// preLen is the length that msg was before the ResourceBody was packed.
+func (h *ResourceHeader) fixLen(msg []byte, lenOff int, preLen int) error {
conLen := len(msg) - preLen
if conLen > int(^uint16(0)) {
return errResTooLong
}
// Fill in the length now that we know how long the content is.
- packUint16(length[:0], uint16(conLen))
+ packUint16(msg[lenOff:lenOff], uint16(conLen))
h.Length = uint16(conLen)
return nil
diff --git a/dns/dnsmessage/message_test.go b/dns/dnsmessage/message_test.go
index d4f3492..25ba8f0 100644
--- a/dns/dnsmessage/message_test.go
+++ b/dns/dnsmessage/message_test.go
@@ -846,6 +846,36 @@
}
}
+func TestResourcePackLength(t *testing.T) {
+ r := Resource{
+ ResourceHeader{
+ Name: MustNewName("."),
+ Type: TypeA,
+ Class: ClassINET,
+ },
+ &AResource{[4]byte{127, 0, 0, 2}},
+ }
+
+ hb, _, err := r.Header.pack(nil, nil, 0)
+ if err != nil {
+ t.Fatal("ResourceHeader.pack() =", err)
+ }
+ buf := make([]byte, 0, len(hb))
+ buf, err = r.pack(buf, nil, 0)
+ if err != nil {
+ t.Fatal("Resource.pack() =", err)
+ }
+
+ var hdr ResourceHeader
+ if _, err := hdr.unpack(buf, 0); err != nil {
+ t.Fatal("ResourceHeader.unpack() =", err)
+ }
+
+ if got, want := int(hdr.Length), len(buf)-len(hb); got != want {
+ t.Errorf("got hdr.Length = %d, want = %d", got, want)
+ }
+}
+
func TestOptionPackUnpack(t *testing.T) {
for _, tt := range []struct {
name string
