Skip to content

Commit 8887df4

Browse files
iangudgerbradfitz
iangudger
authored and
bradfitz
committed
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 <[email protected]> TryBot-Result: Gobot Gobot <[email protected]> Reviewed-by: Brad Fitzpatrick <[email protected]>
1 parent d0887ba commit 8887df4

File tree

2 files changed

+65
-30
lines changed

2 files changed

+65
-30
lines changed

‎dns/dnsmessage/message.go

Lines changed: 35 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -493,7 +493,7 @@ func (r *Resource) pack(msg []byte, compression map[string]int, compressionOff i
493493
}
494494
oldMsg := msg
495495
r.Header.Type = r.Body.realType()
496-
msg, length, err := r.Header.pack(msg, compression, compressionOff)
496+
msg, lenOff, err := r.Header.pack(msg, compression, compressionOff)
497497
if err != nil {
498498
return msg, &nestedError{"ResourceHeader", err}
499499
}
@@ -502,7 +502,7 @@ func (r *Resource) pack(msg []byte, compression map[string]int, compressionOff i
502502
if err != nil {
503503
return msg, &nestedError{"content", err}
504504
}
505-
if err := r.Header.fixLen(msg, length, preLen); err != nil {
505+
if err := r.Header.fixLen(msg, lenOff, preLen); err != nil {
506506
return oldMsg, err
507507
}
508508
return msg, nil
@@ -1323,15 +1323,15 @@ func (b *Builder) CNAMEResource(h ResourceHeader, r CNAMEResource) error {
13231323
return err
13241324
}
13251325
h.Type = r.realType()
1326-
msg, length, err := h.pack(b.msg, b.compression, b.start)
1326+
msg, lenOff, err := h.pack(b.msg, b.compression, b.start)
13271327
if err != nil {
13281328
return &nestedError{"ResourceHeader", err}
13291329
}
13301330
preLen := len(msg)
13311331
if msg, err = r.pack(msg, b.compression, b.start); err != nil {
13321332
return &nestedError{"CNAMEResource body", err}
13331333
}
1334-
if err := h.fixLen(msg, length, preLen); err != nil {
1334+
if err := h.fixLen(msg, lenOff, preLen); err != nil {
13351335
return err
13361336
}
13371337
if err := b.incrementSectionCount(); err != nil {
@@ -1347,15 +1347,15 @@ func (b *Builder) MXResource(h ResourceHeader, r MXResource) error {
13471347
return err
13481348
}
13491349
h.Type = r.realType()
1350-
msg, length, err := h.pack(b.msg, b.compression, b.start)
1350+
msg, lenOff, err := h.pack(b.msg, b.compression, b.start)
13511351
if err != nil {
13521352
return &nestedError{"ResourceHeader", err}
13531353
}
13541354
preLen := len(msg)
13551355
if msg, err = r.pack(msg, b.compression, b.start); err != nil {
13561356
return &nestedError{"MXResource body", err}
13571357
}
1358-
if err := h.fixLen(msg, length, preLen); err != nil {
1358+
if err := h.fixLen(msg, lenOff, preLen); err != nil {
13591359
return err
13601360
}
13611361
if err := b.incrementSectionCount(); err != nil {
@@ -1371,15 +1371,15 @@ func (b *Builder) NSResource(h ResourceHeader, r NSResource) error {
13711371
return err
13721372
}
13731373
h.Type = r.realType()
1374-
msg, length, err := h.pack(b.msg, b.compression, b.start)
1374+
msg, lenOff, err := h.pack(b.msg, b.compression, b.start)
13751375
if err != nil {
13761376
return &nestedError{"ResourceHeader", err}
13771377
}
13781378
preLen := len(msg)
13791379
if msg, err = r.pack(msg, b.compression, b.start); err != nil {
13801380
return &nestedError{"NSResource body", err}
13811381
}
1382-
if err := h.fixLen(msg, length, preLen); err != nil {
1382+
if err := h.fixLen(msg, lenOff, preLen); err != nil {
13831383
return err
13841384
}
13851385
if err := b.incrementSectionCount(); err != nil {
@@ -1395,15 +1395,15 @@ func (b *Builder) PTRResource(h ResourceHeader, r PTRResource) error {
13951395
return err
13961396
}
13971397
h.Type = r.realType()
1398-
msg, length, err := h.pack(b.msg, b.compression, b.start)
1398+
msg, lenOff, err := h.pack(b.msg, b.compression, b.start)
13991399
if err != nil {
14001400
return &nestedError{"ResourceHeader", err}
14011401
}
14021402
preLen := len(msg)
14031403
if msg, err = r.pack(msg, b.compression, b.start); err != nil {
14041404
return &nestedError{"PTRResource body", err}
14051405
}
1406-
if err := h.fixLen(msg, length, preLen); err != nil {
1406+
if err := h.fixLen(msg, lenOff, preLen); err != nil {
14071407
return err
14081408
}
14091409
if err := b.incrementSectionCount(); err != nil {
@@ -1419,15 +1419,15 @@ func (b *Builder) SOAResource(h ResourceHeader, r SOAResource) error {
14191419
return err
14201420
}
14211421
h.Type = r.realType()
1422-
msg, length, err := h.pack(b.msg, b.compression, b.start)
1422+
msg, lenOff, err := h.pack(b.msg, b.compression, b.start)
14231423
if err != nil {
14241424
return &nestedError{"ResourceHeader", err}
14251425
}
14261426
preLen := len(msg)
14271427
if msg, err = r.pack(msg, b.compression, b.start); err != nil {
14281428
return &nestedError{"SOAResource body", err}
14291429
}
1430-
if err := h.fixLen(msg, length, preLen); err != nil {
1430+
if err := h.fixLen(msg, lenOff, preLen); err != nil {
14311431
return err
14321432
}
14331433
if err := b.incrementSectionCount(); err != nil {
@@ -1443,15 +1443,15 @@ func (b *Builder) TXTResource(h ResourceHeader, r TXTResource) error {
14431443
return err
14441444
}
14451445
h.Type = r.realType()
1446-
msg, length, err := h.pack(b.msg, b.compression, b.start)
1446+
msg, lenOff, err := h.pack(b.msg, b.compression, b.start)
14471447
if err != nil {
14481448
return &nestedError{"ResourceHeader", err}
14491449
}
14501450
preLen := len(msg)
14511451
if msg, err = r.pack(msg, b.compression, b.start); err != nil {
14521452
return &nestedError{"TXTResource body", err}
14531453
}
1454-
if err := h.fixLen(msg, length, preLen); err != nil {
1454+
if err := h.fixLen(msg, lenOff, preLen); err != nil {
14551455
return err
14561456
}
14571457
if err := b.incrementSectionCount(); err != nil {
@@ -1467,15 +1467,15 @@ func (b *Builder) SRVResource(h ResourceHeader, r SRVResource) error {
14671467
return err
14681468
}
14691469
h.Type = r.realType()
1470-
msg, length, err := h.pack(b.msg, b.compression, b.start)
1470+
msg, lenOff, err := h.pack(b.msg, b.compression, b.start)
14711471
if err != nil {
14721472
return &nestedError{"ResourceHeader", err}
14731473
}
14741474
preLen := len(msg)
14751475
if msg, err = r.pack(msg, b.compression, b.start); err != nil {
14761476
return &nestedError{"SRVResource body", err}
14771477
}
1478-
if err := h.fixLen(msg, length, preLen); err != nil {
1478+
if err := h.fixLen(msg, lenOff, preLen); err != nil {
14791479
return err
14801480
}
14811481
if err := b.incrementSectionCount(); err != nil {
@@ -1491,15 +1491,15 @@ func (b *Builder) AResource(h ResourceHeader, r AResource) error {
14911491
return err
14921492
}
14931493
h.Type = r.realType()
1494-
msg, length, err := h.pack(b.msg, b.compression, b.start)
1494+
msg, lenOff, err := h.pack(b.msg, b.compression, b.start)
14951495
if err != nil {
14961496
return &nestedError{"ResourceHeader", err}
14971497
}
14981498
preLen := len(msg)
14991499
if msg, err = r.pack(msg, b.compression, b.start); err != nil {
15001500
return &nestedError{"AResource body", err}
15011501
}
1502-
if err := h.fixLen(msg, length, preLen); err != nil {
1502+
if err := h.fixLen(msg, lenOff, preLen); err != nil {
15031503
return err
15041504
}
15051505
if err := b.incrementSectionCount(); err != nil {
@@ -1515,15 +1515,15 @@ func (b *Builder) AAAAResource(h ResourceHeader, r AAAAResource) error {
15151515
return err
15161516
}
15171517
h.Type = r.realType()
1518-
msg, length, err := h.pack(b.msg, b.compression, b.start)
1518+
msg, lenOff, err := h.pack(b.msg, b.compression, b.start)
15191519
if err != nil {
15201520
return &nestedError{"ResourceHeader", err}
15211521
}
15221522
preLen := len(msg)
15231523
if msg, err = r.pack(msg, b.compression, b.start); err != nil {
15241524
return &nestedError{"AAAAResource body", err}
15251525
}
1526-
if err := h.fixLen(msg, length, preLen); err != nil {
1526+
if err := h.fixLen(msg, lenOff, preLen); err != nil {
15271527
return err
15281528
}
15291529
if err := b.incrementSectionCount(); err != nil {
@@ -1539,15 +1539,15 @@ func (b *Builder) OPTResource(h ResourceHeader, r OPTResource) error {
15391539
return err
15401540
}
15411541
h.Type = r.realType()
1542-
msg, length, err := h.pack(b.msg, b.compression, b.start)
1542+
msg, lenOff, err := h.pack(b.msg, b.compression, b.start)
15431543
if err != nil {
15441544
return &nestedError{"ResourceHeader", err}
15451545
}
15461546
preLen := len(msg)
15471547
if msg, err = r.pack(msg, b.compression, b.start); err != nil {
15481548
return &nestedError{"OPTResource body", err}
15491549
}
1550-
if err := h.fixLen(msg, length, preLen); err != nil {
1550+
if err := h.fixLen(msg, lenOff, preLen); err != nil {
15511551
return err
15521552
}
15531553
if err := b.incrementSectionCount(); err != nil {
@@ -1606,19 +1606,18 @@ func (h *ResourceHeader) GoString() string {
16061606

16071607
// pack appends the wire format of the ResourceHeader to oldMsg.
16081608
//
1609-
// The bytes where length was packed are returned as a slice so they can be
1610-
// updated after the rest of the Resource has been packed.
1611-
func (h *ResourceHeader) pack(oldMsg []byte, compression map[string]int, compressionOff int) (msg []byte, length []byte, err error) {
1609+
// lenOff is the offset in msg where the Length field was packed.
1610+
func (h *ResourceHeader) pack(oldMsg []byte, compression map[string]int, compressionOff int) (msg []byte, lenOff int, err error) {
16121611
msg = oldMsg
16131612
if msg, err = h.Name.pack(msg, compression, compressionOff); err != nil {
1614-
return oldMsg, nil, &nestedError{"Name", err}
1613+
return oldMsg, 0, &nestedError{"Name", err}
16151614
}
16161615
msg = packType(msg, h.Type)
16171616
msg = packClass(msg, h.Class)
16181617
msg = packUint32(msg, h.TTL)
1619-
lenBegin := len(msg)
1618+
lenOff = len(msg)
16201619
msg = packUint16(msg, h.Length)
1621-
return msg, msg[lenBegin : lenBegin+uint16Len], nil
1620+
return msg, lenOff, nil
16221621
}
16231622

16241623
func (h *ResourceHeader) unpack(msg []byte, off int) (int, error) {
@@ -1642,14 +1641,20 @@ func (h *ResourceHeader) unpack(msg []byte, off int) (int, error) {
16421641
return newOff, nil
16431642
}
16441643

1645-
func (h *ResourceHeader) fixLen(msg []byte, length []byte, preLen int) error {
1644+
// fixLen updates a packed ResourceHeader to include the length of the
1645+
// ResourceBody.
1646+
//
1647+
// lenOff is the offset of the ResourceHeader.Length field in msg.
1648+
//
1649+
// preLen is the length that msg was before the ResourceBody was packed.
1650+
func (h *ResourceHeader) fixLen(msg []byte, lenOff int, preLen int) error {
16461651
conLen := len(msg) - preLen
16471652
if conLen > int(^uint16(0)) {
16481653
return errResTooLong
16491654
}
16501655

16511656
// Fill in the length now that we know how long the content is.
1652-
packUint16(length[:0], uint16(conLen))
1657+
packUint16(msg[lenOff:lenOff], uint16(conLen))
16531658
h.Length = uint16(conLen)
16541659

16551660
return nil

‎dns/dnsmessage/message_test.go

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -846,6 +846,36 @@ func TestResourcePack(t *testing.T) {
846846
}
847847
}
848848

849+
func TestResourcePackLength(t *testing.T) {
850+
r := Resource{
851+
ResourceHeader{
852+
Name: MustNewName("."),
853+
Type: TypeA,
854+
Class: ClassINET,
855+
},
856+
&AResource{[4]byte{127, 0, 0, 2}},
857+
}
858+
859+
hb, _, err := r.Header.pack(nil, nil, 0)
860+
if err != nil {
861+
t.Fatal("ResourceHeader.pack() =", err)
862+
}
863+
buf := make([]byte, 0, len(hb))
864+
buf, err = r.pack(buf, nil, 0)
865+
if err != nil {
866+
t.Fatal("Resource.pack() =", err)
867+
}
868+
869+
var hdr ResourceHeader
870+
if _, err := hdr.unpack(buf, 0); err != nil {
871+
t.Fatal("ResourceHeader.unpack() =", err)
872+
}
873+
874+
if got, want := int(hdr.Length), len(buf)-len(hb); got != want {
875+
t.Errorf("got hdr.Length = %d, want = %d", got, want)
876+
}
877+
}
878+
849879
func TestOptionPackUnpack(t *testing.T) {
850880
for _, tt := range []struct {
851881
name string

0 commit comments

Comments
 (0)