From 577c6822b78b2beeca249ab754bf46524a69abad Mon Sep 17 00:00:00 2001 From: Jukka Taimisto Date: Mon, 14 Feb 2022 11:50:24 +0200 Subject: [PATCH 01/13] Pass payload only up to end of RDATA when parsing resource record Since RDATA can contain variable-length data, this ensures we will not read past the end of RDATA buffer when parsing it. Compressed labels can contain pointers only to prior data, thus full payload is not needed for parsing labels. Only the prior data. --- dnsutils/dns.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dnsutils/dns.go b/dnsutils/dns.go index f6fa8de5..c00e780b 100644 --- a/dnsutils/dns.go +++ b/dnsutils/dns.go @@ -304,7 +304,7 @@ func DecodeAnswer(ancount int, start_offset int, payload []byte) ([]DnsAnswer, i } // parse rdata rdatatype := RdatatypeToString(int(t)) - parsed, err := ParseRdata(rdatatype, rdata, payload, offset_next+10) + parsed, err := ParseRdata(rdatatype, rdata, payload[:offset_next+10+int(rdlength)], offset_next+10) if err != nil { return answers, offset, err } From 55c279ee6ef8e15e034052a727a8a3bfa0e5d05e Mon Sep 17 00:00:00 2001 From: Jukka Taimisto Date: Tue, 15 Feb 2022 11:43:47 +0200 Subject: [PATCH 02/13] Ensure there is enough data on RDATA to parse SOA record --- dnsutils/dns.go | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/dnsutils/dns.go b/dnsutils/dns.go index c00e780b..fa5cd8b6 100644 --- a/dnsutils/dns.go +++ b/dnsutils/dns.go @@ -463,7 +463,12 @@ func ParseSOA(rdata_offset int, payload []byte) (string, error) { if err != nil { return "", err } - rdata := payload[offset:] + + // ensure there is enough data to parse rest of the fields + if offset+20 > len(payload) { + return "", ErrDecodeDnsAnswerRdataTooShort + } + rdata := payload[offset : offset+20] serial := binary.BigEndian.Uint32(rdata[0:4]) refresh := int32(binary.BigEndian.Uint32(rdata[4:8])) From e0e21d51e0d59a98884d59c22ec98fc930afb48e Mon Sep 17 00:00:00 2001 From: Jukka Taimisto Date: Tue, 15 Feb 2022 11:44:10 +0200 Subject: [PATCH 03/13] Add unit test to verify too short SOA RDATA does not cause panic --- dnsutils/dns_test.go | 52 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 52 insertions(+) diff --git a/dnsutils/dns_test.go b/dnsutils/dns_test.go index 9c227ff1..3776e2df 100644 --- a/dnsutils/dns_test.go +++ b/dnsutils/dns_test.go @@ -313,6 +313,58 @@ func TestDecodeRdataSOA(t *testing.T) { } } +func TestDecodeRdataSOA_Short(t *testing.T) { + payload := []byte{ + // header + 0x28, 0xba, 0x01, 0x00, 0x00, 0x01, 0x00, 0x01, + 0x00, 0x00, 0x00, 0x00, + // Query section + 0x0f, 0x64, 0x6e, 0x73, + 0x74, 0x61, 0x70, 0x63, 0x6f, 0x6c, 0x6c, 0x65, + 0x63, 0x74, 0x6f, 0x72, 0x04, 0x74, 0x65, 0x73, + 0x74, 0x00, 0x00, 0x01, 0x00, 0x01, + // Answer Resource Record, + 0x0f, 0x64, + 0x6e, 0x73, 0x74, 0x61, 0x70, 0x63, 0x6f, 0x6c, + 0x6c, 0x65, 0x63, 0x74, 0x6f, 0x72, 0x04, 0x74, + 0x65, 0x73, 0x74, 0x00, + // type SOA, class IN + 0x00, 0x06, 0x00, 0x01, + // TTL + 0x00, 0x00, 0x0e, 0x10, + // RDLENGTH 54 + 0x00, 0x36, + // RDATA + // MNAME + 0x03, 0x6e, + 0x73, 0x31, 0x06, 0x67, 0x6f, 0x6f, 0x67, 0x6c, + 0x65, 0x03, 0x63, 0x6f, 0x6d, 0x00, + // RNAME + 0x09, 0x64, + 0x6e, 0x73, 0x2d, 0x61, 0x64, 0x6d, 0x69, 0x6e, + 0x06, 0x67, 0x6f, 0x6f, 0x67, 0x6c, 0x65, 0x03, + 0x63, 0x6f, 0x6d, 0x00, + // serial + 0x18, 0x94, 0xea, 0xef, + // refresh + 0x00, 0x00, 0x03, 0x84, + // retry + 0x00, 0x00, 0x03, 0x84, + // expire + 0x00, 0x00, 0x07, 0x08, + // minimum -field missing from the RDATA + } + + _, _, offsset_rr, err := DecodeQuestion(payload) + if err != nil { + t.Errorf("Unable to decode question: %v", err) + } + _, _, erra := DecodeAnswer(1, offsset_rr, payload) + if !errors.Is(erra, ErrDecodeDnsAnswerRdataTooShort) { + t.Errorf("bad error returned: %v", erra) + } +} + func TestDecodeRdataSOA_Minimization(t *testing.T) { // loop between qnames payload := []byte{164, 66, 129, 128, 0, 1, 0, 0, 0, 1, 0, 0, 8, 102, 114, 101, 115, 104, 114, 115, 115, 4, 109, From f823a6b32c8173e1bb8733454d50d995e8d67f29 Mon Sep 17 00:00:00 2001 From: Jukka Taimisto Date: Tue, 15 Feb 2022 12:58:06 +0200 Subject: [PATCH 04/13] Ensure there is enough data on RDATA to parse A or AAAA record Use net.IP to format the textual representation of addresses. --- dnsutils/dns.go | 23 ++++++++++++----------- dnsutils/dns_test.go | 2 +- 2 files changed, 13 insertions(+), 12 deletions(-) diff --git a/dnsutils/dns.go b/dnsutils/dns.go index fa5cd8b6..aa81f382 100644 --- a/dnsutils/dns.go +++ b/dnsutils/dns.go @@ -4,7 +4,7 @@ import ( "encoding/binary" "errors" "fmt" - "strconv" + "net" "strings" ) @@ -490,12 +490,13 @@ IPv4 +--+--+--+--+--+--+--+--+--+--+--+--+--+--+--+--+ */ func ParseA(r []byte) (string, error) { - var ip []string - for i := 0; i < len(r); i++ { - ip = append(ip, strconv.Itoa(int(r[i]))) + + if len(r) < net.IPv4len { + return "", ErrDecodeDnsAnswerRdataTooShort } - a := strings.Join(ip, ".") - return a, nil + addr := make(net.IP, net.IPv4len) + copy(addr, r[:net.IPv4len]) + return addr.String(), nil } /* @@ -514,12 +515,12 @@ IPv6 +--+--+--+--+--+--+--+--+--+--+--+--+--+--+--+--+ */ func ParseAAAA(rdata []byte) (string, error) { - var ip []string - for i := 0; i < len(rdata); i += 2 { - ip = append(ip, fmt.Sprintf("%x", binary.BigEndian.Uint16(rdata[i:i+2]))) + if len(rdata) < net.IPv6len { + return "", ErrDecodeDnsAnswerRdataTooShort } - aaaa := strings.Join(ip, ":") - return aaaa, nil + addr := make(net.IP, net.IPv6len) + copy(addr, rdata[:net.IPv6len]) + return addr.String(), nil } /* diff --git a/dnsutils/dns_test.go b/dnsutils/dns_test.go index 3776e2df..f4c85e75 100644 --- a/dnsutils/dns_test.go +++ b/dnsutils/dns_test.go @@ -159,7 +159,7 @@ func TestDecodeRdataAAAA(t *testing.T) { dm := new(dns.Msg) dm.SetQuestion(fqdn, dns.TypeA) - rdata := "fe80:0:0:0:0:0:0:2" + rdata := "fe8::2" rr1, _ := dns.NewRR(fmt.Sprintf("%s AAAA %s", fqdn, rdata)) dm.Answer = append(dm.Answer, rr1) From 431718da375ffe5f5a4432c67edc6ff95d78ff8a Mon Sep 17 00:00:00 2001 From: Jukka Taimisto Date: Tue, 15 Feb 2022 12:58:38 +0200 Subject: [PATCH 05/13] Add unit tests to verify short RDATA for A and AAAA records is handled --- dnsutils/dns_test.go | 70 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 70 insertions(+) diff --git a/dnsutils/dns_test.go b/dnsutils/dns_test.go index f4c85e75..ef844fd4 100644 --- a/dnsutils/dns_test.go +++ b/dnsutils/dns_test.go @@ -153,6 +153,40 @@ func TestDecodeRdataA(t *testing.T) { } } +func TestDecodeRdataA_Short(t *testing.T) { + payload := []byte{ + 0x43, 0xac, 0x01, 0x00, 0x00, 0x01, 0x00, 0x01, + 0x00, 0x00, 0x00, 0x00, + // Query section + 0x0f, 0x64, 0x6e, 0x73, + 0x74, 0x61, 0x70, 0x63, 0x6f, 0x6c, 0x6c, 0x65, + 0x63, 0x74, 0x6f, 0x72, 0x04, 0x74, 0x65, 0x73, + 0x74, 0x00, 0x00, 0x01, 0x00, 0x01, + // Answer Resource Record + 0x0f, 0x64, + 0x6e, 0x73, 0x74, 0x61, 0x70, 0x63, 0x6f, 0x6c, + 0x6c, 0x65, 0x63, 0x74, 0x6f, 0x72, 0x04, 0x74, + 0x65, 0x73, 0x74, 0x00, + // type A, class IN + 0x00, 0x01, 0x00, 0x01, + // TTL + 0x00, 0x00, 0x0e, 0x10, + // RDLENGTH + 0x00, 0x03, + // RDATA (1 byte too short for A record) + 0x7f, 0x00, 0x00, + } + _, _, offsetrr, err := DecodeQuestion(payload) + if err != nil { + t.Errorf("Unexpected error decoding question: %v", err) + } + + _, _, erra := DecodeAnswer(1, offsetrr, payload) + if !errors.Is(erra, ErrDecodeDnsAnswerRdataTooShort) { + t.Errorf("bad error returned: %v", erra) + } +} + func TestDecodeRdataAAAA(t *testing.T) { fqdn := "dnstapcollector.test." @@ -172,6 +206,42 @@ func TestDecodeRdataAAAA(t *testing.T) { t.Errorf("invalid decode for rdata AAAA, want %s, got: %s", rdata, answer[0].Rdata) } } +func TestDecodeRdataAAAA_Short(t *testing.T) { + payload := []byte{ + // header + 0x3b, 0x33, 0x01, 0x00, 0x00, 0x01, 0x00, 0x01, + 0x00, 0x00, 0x00, 0x00, + // Query section + 0x0f, 0x64, 0x6e, 0x73, + 0x74, 0x61, 0x70, 0x63, 0x6f, 0x6c, 0x6c, 0x65, + 0x63, 0x74, 0x6f, 0x72, 0x04, 0x74, 0x65, 0x73, + 0x74, 0x00, 0x00, 0x01, 0x00, 0x01, + // Answer resource record + 0x0f, 0x64, + 0x6e, 0x73, 0x74, 0x61, 0x70, 0x63, 0x6f, 0x6c, + 0x6c, 0x65, 0x63, 0x74, 0x6f, 0x72, 0x04, 0x74, + 0x65, 0x73, 0x74, 0x00, + // type AAAA, class IN + 0x00, 0x1c, 0x00, 0x01, + // TTL + 0x00, 0x00, 0x0e, 0x10, + // RDLENGTH + 0x00, 0x0c, + // RDATA + 0xfe, 0x80, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, + } + + _, _, offsetset_rr, err := DecodeQuestion(payload) + if err != nil { + t.Errorf("unexpected error while decoding question: %v", err) + } + _, _, erra := DecodeAnswer(1, offsetset_rr, payload) + if !errors.Is(erra, ErrDecodeDnsAnswerRdataTooShort) { + t.Errorf("bad error returned: %v", erra) + } +} func TestDecodeRdataCNAME(t *testing.T) { fqdn := "dnstapcollector.test." From b75abef5101ecabe29d6ceb424c330c981608133 Mon Sep 17 00:00:00 2001 From: Jukka Taimisto Date: Tue, 15 Feb 2022 14:01:26 +0200 Subject: [PATCH 06/13] Ensure there is enough data in RDATA to parse MX record --- dnsutils/dns.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/dnsutils/dns.go b/dnsutils/dns.go index aa81f382..11e9e632 100644 --- a/dnsutils/dns.go +++ b/dnsutils/dns.go @@ -552,11 +552,17 @@ MX +--+--+--+--+--+--+--+--+--+--+--+--+--+--+--+--+ */ func ParseMX(rdata_offset int, payload []byte) (string, error) { + // ensure there is enough data for pereference and at least + // one byte for label + if len(payload) < rdata_offset+3 { + return "", ErrDecodeDnsAnswerRdataTooShort + } pref := binary.BigEndian.Uint16(payload[rdata_offset : rdata_offset+2]) host, _, err := ParseLabels(rdata_offset+2, payload) if err != nil { return "", err } + mx := fmt.Sprintf("%d %s", pref, host) return mx, err } From 47a13d45925c7667e8c3243c512330b3604b934e Mon Sep 17 00:00:00 2001 From: Jukka Taimisto Date: Tue, 15 Feb 2022 14:02:22 +0200 Subject: [PATCH 07/13] Add unit tests to verify short MX record is parsed properly --- dnsutils/dns_test.go | 73 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 73 insertions(+) diff --git a/dnsutils/dns_test.go b/dnsutils/dns_test.go index ef844fd4..c79728bb 100644 --- a/dnsutils/dns_test.go +++ b/dnsutils/dns_test.go @@ -283,6 +283,79 @@ func TestDecodeRdataMX(t *testing.T) { } } +func TestDecodeRdataMX_Short(t *testing.T) { + payload := []byte{ + // header + 0xed, 0x7f, 0x01, 0x00, 0x00, 0x01, 0x00, 0x01, + 0x00, 0x00, 0x00, 0x00, + // Question seection + 0x0f, 0x64, 0x6e, 0x73, + 0x74, 0x61, 0x70, 0x63, 0x6f, 0x6c, 0x6c, 0x65, + 0x63, 0x74, 0x6f, 0x72, 0x04, 0x74, 0x65, 0x73, + 0x74, 0x00, 0x00, 0x01, 0x00, 0x01, + // Answer Resource Record + 0x0f, 0x64, + 0x6e, 0x73, 0x74, 0x61, 0x70, 0x63, 0x6f, 0x6c, + 0x6c, 0x65, 0x63, 0x74, 0x6f, 0x72, 0x04, 0x74, + 0x65, 0x73, 0x74, 0x00, + // type MX, class IN + 0x00, 0x0f, 0x00, 0x01, + // TTL + 0x00, 0x00, 0x0e, 0x10, + // RDLENGTH + 0x00, 0x01, + // RDATA + 0x00, + } + _, _, offset_rr, err := DecodeQuestion(payload) + if err != nil { + t.Errorf("unexpected error while decoding question: %v", err) + } + _, _, erra := DecodeAnswer(1, offset_rr, payload) + if !errors.Is(erra, ErrDecodeDnsAnswerRdataTooShort) { + t.Errorf("bad error returned: %v", erra) + } + +} + +func TestDecodeRdataMX_Minimal(t *testing.T) { + payload := []byte{ + // header + 0xed, 0x7f, 0x01, 0x00, 0x00, 0x01, 0x00, 0x01, + 0x00, 0x00, 0x00, 0x00, + // Question seection + 0x0f, 0x64, 0x6e, 0x73, + 0x74, 0x61, 0x70, 0x63, 0x6f, 0x6c, 0x6c, 0x65, + 0x63, 0x74, 0x6f, 0x72, 0x04, 0x74, 0x65, 0x73, + 0x74, 0x00, 0x00, 0x01, 0x00, 0x01, + // Answer Resource Record + 0x0f, 0x64, + 0x6e, 0x73, 0x74, 0x61, 0x70, 0x63, 0x6f, 0x6c, + 0x6c, 0x65, 0x63, 0x74, 0x6f, 0x72, 0x04, 0x74, + 0x65, 0x73, 0x74, 0x00, + // type MX, class IN + 0x00, 0x0f, 0x00, 0x01, + // TTL + 0x00, 0x00, 0x0e, 0x10, + // RDLENGTH + 0x00, 0x03, + // RDATA + 0x00, 0x00, 0x00, + } + _, _, offset_rr, err := DecodeQuestion(payload) + if err != nil { + t.Errorf("unexpected error while decoding question: %v", err) + } + answer, _, erra := DecodeAnswer(1, offset_rr, payload) + if erra != nil { + t.Errorf("unexpected error while decoding answer: %v", err) + } + expected := "0 " + if answer[0].Rdata != expected { + t.Errorf("invalid decode for MX rdata, expected %s got %s", expected, answer[0].Rdata) + } +} + func TestDecodeRdataSRV(t *testing.T) { fqdn := "dnstapcollector.test." From e3e08a924cbe4f2070227f20d0b982f76b7187bb Mon Sep 17 00:00:00 2001 From: Jukka Taimisto Date: Thu, 17 Feb 2022 11:36:20 +0200 Subject: [PATCH 08/13] Ensure there is enough data in RDATA to parse SRV record --- dnsutils/dns.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/dnsutils/dns.go b/dnsutils/dns.go index 11e9e632..299c3adf 100644 --- a/dnsutils/dns.go +++ b/dnsutils/dns.go @@ -582,6 +582,9 @@ SRV +--+--+--+--+--+--+--+--+--+--+--+--+--+--+--+--+ */ func ParseSRV(rdata_offset int, payload []byte) (string, error) { + if len(payload) < rdata_offset+7 { + return "", ErrDecodeDnsAnswerRdataTooShort + } priority := binary.BigEndian.Uint16(payload[rdata_offset : rdata_offset+2]) weight := binary.BigEndian.Uint16(payload[rdata_offset+2 : rdata_offset+4]) port := binary.BigEndian.Uint16(payload[rdata_offset+4 : rdata_offset+6]) From 13622d0feffe7e6f4bbaf7ae65a9df27dd8cf94b Mon Sep 17 00:00:00 2001 From: Jukka Taimisto Date: Thu, 17 Feb 2022 11:51:20 +0200 Subject: [PATCH 09/13] Add unit test to verify short SRV record is parsed properly --- dnsutils/dns_test.go | 84 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 84 insertions(+) diff --git a/dnsutils/dns_test.go b/dnsutils/dns_test.go index c79728bb..9464a208 100644 --- a/dnsutils/dns_test.go +++ b/dnsutils/dns_test.go @@ -376,6 +376,90 @@ func TestDecodeRdataSRV(t *testing.T) { } } +func TestDecodeRdataSRV_Short(t *testing.T) { + payload := []byte{ + // header + 0xd9, 0x93, 0x01, 0x00, 0x00, 0x01, 0x00, 0x01, + 0x00, 0x00, 0x00, 0x00, + // Question section + 0x0f, 0x64, 0x6e, 0x73, + 0x74, 0x61, 0x70, 0x63, 0x6f, 0x6c, 0x6c, 0x65, + 0x63, 0x74, 0x6f, 0x72, 0x04, 0x74, 0x65, 0x73, + 0x74, 0x00, 0x00, 0x01, 0x00, 0x01, + // Answer section + 0x0f, 0x64, + 0x6e, 0x73, 0x74, 0x61, 0x70, 0x63, 0x6f, 0x6c, + 0x6c, 0x65, 0x63, 0x74, 0x6f, 0x72, 0x04, 0x74, + 0x65, 0x73, 0x74, 0x00, + // type SRV, class IN + 0x00, 0x21, 0x00, 0x01, + // TTL + 0x00, 0x00, 0x0e, 0x10, + // RDLENGTH + 0x00, 0x04, + // RDATA + // priority + 0x00, 0x14, + // weight + 0x00, 0x00, + // missing port and target + } + + _, _, offset_rr, err := DecodeQuestion(payload) + if err != nil { + t.Errorf("unexpected error while decoding question: %v", err) + } + _, _, erra := DecodeAnswer(1, offset_rr, payload) + if !errors.Is(erra, ErrDecodeDnsAnswerRdataTooShort) { + t.Errorf("bad error returned: %v", erra) + } +} + +func TestDecodeRdataSRV_Minimal(t *testing.T) { + payload := []byte{ + // header + 0xd9, 0x93, 0x01, 0x00, 0x00, 0x01, 0x00, 0x01, + 0x00, 0x00, 0x00, 0x00, + // Question section + 0x0f, 0x64, 0x6e, 0x73, + 0x74, 0x61, 0x70, 0x63, 0x6f, 0x6c, 0x6c, 0x65, + 0x63, 0x74, 0x6f, 0x72, 0x04, 0x74, 0x65, 0x73, + 0x74, 0x00, 0x00, 0x01, 0x00, 0x01, + // Answer section + 0x0f, 0x64, + 0x6e, 0x73, 0x74, 0x61, 0x70, 0x63, 0x6f, 0x6c, + 0x6c, 0x65, 0x63, 0x74, 0x6f, 0x72, 0x04, 0x74, + 0x65, 0x73, 0x74, 0x00, + // type SRV, class IN + 0x00, 0x21, 0x00, 0x01, + // TTL + 0x00, 0x00, 0x0e, 0x10, + // RDLENGTH + 0x00, 0x07, + // RDATA + // priority + 0x00, 0x14, + // weight + 0x00, 0x00, + // port + 0x00, 0x10, + // empty target + 0x00, + } + + _, _, offset_rr, err := DecodeQuestion(payload) + if err != nil { + t.Errorf("unexpected error while decoding question: %v", err) + } + answer, _, erra := DecodeAnswer(1, offset_rr, payload) + if erra != nil { + t.Errorf("unexpected error while decoding answer: %v", err) + } + expected_rdata := "20 0 16 " + if answer[0].Rdata != expected_rdata { + t.Errorf("invalid decode for rdata SRV, want %s, got: %s", expected_rdata, answer[0].Rdata) + } +} func TestDecodeRdataNS(t *testing.T) { fqdn := "dnstapcollector.test." From c9996d12644be004275601c60cff336a80f6e348 Mon Sep 17 00:00:00 2001 From: Jukka Taimisto Date: Thu, 17 Feb 2022 13:23:33 +0200 Subject: [PATCH 10/13] Ensure there is enough data in RDATA to parse TXT record --- dnsutils/dns.go | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/dnsutils/dns.go b/dnsutils/dns.go index 299c3adf..ee3e97c5 100644 --- a/dnsutils/dns.go +++ b/dnsutils/dns.go @@ -624,7 +624,14 @@ TXT +--+--+--+--+--+--+--+--+--+--+--+--+--+--+--+--+ */ func ParseTXT(rdata []byte) (string, error) { + // ensure there is enough data to read the length + if len(rdata) < 1 { + return "", ErrDecodeDnsAnswerRdataTooShort + } length := int(rdata[0]) + if len(rdata)-1 < length { + return "", ErrDecodeDnsAnswerRdataTooShort + } txt := string(rdata[1 : length+1]) return txt, nil } From 27d318a1645da8be96e8393ca3f3dfa7cf72def4 Mon Sep 17 00:00:00 2001 From: Jukka Taimisto Date: Thu, 17 Feb 2022 13:24:32 +0200 Subject: [PATCH 11/13] Add unit tests to verify TXT record is parsed properly --- dnsutils/dns_test.go | 116 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 116 insertions(+) diff --git a/dnsutils/dns_test.go b/dnsutils/dns_test.go index 9464a208..aa02df07 100644 --- a/dnsutils/dns_test.go +++ b/dnsutils/dns_test.go @@ -500,6 +500,122 @@ func TestDecodeRdataTXT(t *testing.T) { } } +func TestDecodeRdataTXT_Empty(t *testing.T) { + payload := []byte{ + // header + 0x86, 0x08, 0x01, 0x00, 0x00, 0x01, 0x00, 0x01, + 0x00, 0x00, 0x00, 0x00, + // question section + 0x0f, 0x64, 0x6e, 0x73, + 0x74, 0x61, 0x70, 0x63, 0x6f, 0x6c, 0x6c, 0x65, + 0x63, 0x74, 0x6f, 0x72, 0x04, 0x74, 0x65, 0x73, + 0x74, 0x00, 0x00, 0x01, 0x00, 0x01, + // answer section + 0x0f, 0x64, + 0x6e, 0x73, 0x74, 0x61, 0x70, 0x63, 0x6f, 0x6c, + 0x6c, 0x65, 0x63, 0x74, 0x6f, 0x72, 0x04, 0x74, + 0x65, 0x73, 0x74, 0x00, + // type TXT, class IN + 0x00, 0x10, 0x00, 0x01, + // TTL + 0x00, 0x00, 0x0e, 0x10, + // RDLENGTH + 0x00, 0x00, + // no data + } + + _, _, offset_rr, err := DecodeQuestion(payload) + if err != nil { + t.Errorf("unexpected error while decoding question: %v", err) + } + _, _, erra := DecodeAnswer(1, offset_rr, payload) + if !errors.Is(erra, ErrDecodeDnsAnswerRdataTooShort) { + t.Errorf("bad error returned: %v", erra) + } + +} +func TestDecodeRdataTXT_Short(t *testing.T) { + payload := []byte{ + // header + 0x86, 0x08, 0x01, 0x00, 0x00, 0x01, 0x00, 0x01, + 0x00, 0x00, 0x00, 0x00, + // question section + 0x0f, 0x64, 0x6e, 0x73, + 0x74, 0x61, 0x70, 0x63, 0x6f, 0x6c, 0x6c, 0x65, + 0x63, 0x74, 0x6f, 0x72, 0x04, 0x74, 0x65, 0x73, + 0x74, 0x00, 0x00, 0x01, 0x00, 0x01, + // answer section + 0x0f, 0x64, + 0x6e, 0x73, 0x74, 0x61, 0x70, 0x63, 0x6f, 0x6c, + 0x6c, 0x65, 0x63, 0x74, 0x6f, 0x72, 0x04, 0x74, + 0x65, 0x73, 0x74, 0x00, + // type TXT, class IN + 0x00, 0x10, 0x00, 0x01, + // TTL + 0x00, 0x00, 0x0e, 0x10, + // RDLENGTH + 0x00, 0x0a, + // RDATA + // length + 0x0b, + // characters + 0x68, + 0x65, 0x6c, 0x6c, 0x6f, 0x20, 0x77, 0x6f, 0x72, + // missing two bytes + } + + _, _, offset_rr, err := DecodeQuestion(payload) + if err != nil { + t.Errorf("unexpected error while decoding question: %v", err) + } + _, _, erra := DecodeAnswer(1, offset_rr, payload) + if !errors.Is(erra, ErrDecodeDnsAnswerRdataTooShort) { + t.Errorf("bad error returned: %v", erra) + } + +} +func TestDecodeRdataTXT_NoTxt(t *testing.T) { + payload := []byte{ + // header + 0x86, 0x08, 0x01, 0x00, 0x00, 0x01, 0x00, 0x01, + 0x00, 0x00, 0x00, 0x00, + // question section + 0x0f, 0x64, 0x6e, 0x73, + 0x74, 0x61, 0x70, 0x63, 0x6f, 0x6c, 0x6c, 0x65, + 0x63, 0x74, 0x6f, 0x72, 0x04, 0x74, 0x65, 0x73, + 0x74, 0x00, 0x00, 0x01, 0x00, 0x01, + // answer section + 0x0f, 0x64, + 0x6e, 0x73, 0x74, 0x61, 0x70, 0x63, 0x6f, 0x6c, + 0x6c, 0x65, 0x63, 0x74, 0x6f, 0x72, 0x04, 0x74, + 0x65, 0x73, 0x74, 0x00, + // type TXT, class IN + 0x00, 0x10, 0x00, 0x01, + // TTL + 0x00, 0x00, 0x0e, 0x10, + // RDLENGTH + 0x00, 0x01, + // RDATA + // length + 0x00, + // no txt-data + } + + _, _, offset_rr, err := DecodeQuestion(payload) + if err != nil { + t.Errorf("unexpected error while decoding question: %v", err) + } + answer, _, erra := DecodeAnswer(1, offset_rr, payload) + if erra != nil { + t.Errorf("unexpected error while decoding answer: %v", err) + } + + if answer[0].Rdata != "" { + t.Errorf("expected empty string in RDATA, got: %s", answer[0].Rdata) + } + +} + func TestDecodeRdataPTR(t *testing.T) { fqdn := "dnstapcollector.test." From 408f0a946d98b1ed0dcef226dcfcfb66f364fedf Mon Sep 17 00:00:00 2001 From: Jukka Taimisto Date: Thu, 17 Feb 2022 14:29:50 +0200 Subject: [PATCH 12/13] Update offset when ignoring OPT record Offset needs to be updated to allow us continue parsing the next recrods. --- dnsutils/dns.go | 1 + 1 file changed, 1 insertion(+) diff --git a/dnsutils/dns.go b/dnsutils/dns.go index ee3e97c5..30ac0cc8 100644 --- a/dnsutils/dns.go +++ b/dnsutils/dns.go @@ -300,6 +300,7 @@ func DecodeAnswer(ancount int, start_offset int, payload []byte) ([]DnsAnswer, i // ignore OPT, this type is decoded in the EDNS extension if t == 41 { + offset = offset_next + 10 + int(rdlength) continue } // parse rdata From c8773151c85620688029d66536ed8d47b42ef2c6 Mon Sep 17 00:00:00 2001 From: Jukka Taimisto Date: Thu, 17 Feb 2022 14:30:16 +0200 Subject: [PATCH 13/13] Add unit test to verify OPT record is skipped and rest are read --- dnsutils/dns_test.go | 52 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 52 insertions(+) diff --git a/dnsutils/dns_test.go b/dnsutils/dns_test.go index aa02df07..3c9f38bc 100644 --- a/dnsutils/dns_test.go +++ b/dnsutils/dns_test.go @@ -721,6 +721,58 @@ func TestDecodeRdataSOA_Minimization(t *testing.T) { t.Errorf(" error returned: %v", err) } } +func TestDecodeQuestion_SkipOpt(t *testing.T) { + payload := []byte{ + 0x43, 0xac, 0x01, 0x00, 0x00, 0x01, 0x00, 0x02, + 0x00, 0x00, 0x00, 0x00, + // Query section + 0x0f, 0x64, 0x6e, 0x73, + 0x74, 0x61, 0x70, 0x63, 0x6f, 0x6c, 0x6c, 0x65, + 0x63, 0x74, 0x6f, 0x72, 0x04, 0x74, 0x65, 0x73, + 0x74, 0x00, 0x00, 0x01, 0x00, 0x01, + // Answer Resource Records + 0x0f, 0x64, + 0x6e, 0x73, 0x74, 0x61, 0x70, 0x63, 0x6f, 0x6c, + 0x6c, 0x65, 0x63, 0x74, 0x6f, 0x72, 0x04, 0x74, + 0x65, 0x73, 0x74, 0x00, + // type OPT, class IN + 0x00, 0x29, 0x00, 0x01, + // TTL + 0x00, 0x00, 0x0e, 0x10, + // RDLENGTH + 0x00, 0x01, + //RDATA + 0x01, + // 2nd resource record + 0x0f, 0x64, + 0x6e, 0x73, 0x74, 0x61, 0x70, 0x63, 0x6f, 0x6c, + 0x6c, 0x65, 0x63, 0x74, 0x6f, 0x72, 0x04, 0x74, + 0x65, 0x73, 0x74, 0x00, + // type A, class IN + 0x00, 0x01, 0x00, 0x01, + // TTL + 0x00, 0x00, 0x0e, 0x10, + // RDLENGTH + 0x00, 0x04, + // RDATA + 0x7f, 0x00, 0x00, 0x01, + } + _, _, offsetrr, err := DecodeQuestion(payload) + if err != nil { + t.Errorf("Unexpected error decoding question: %v", err) + } + + answer, _, erra := DecodeAnswer(2, offsetrr, payload) + if erra != nil { + t.Errorf("Unexpected error decoding answer: %v", erra) + } + if len(answer) != 1 { + t.Fatalf("Expected answer to contain one resource record, got %d", len(answer)) + } + if answer[0].Rdatatype != RdatatypeToString(0x01) || answer[0].Rdata != "127.0.0.1" { + t.Errorf("unexpected answer %s %s, expected A 127.0.0.1", answer[0].Rdatatype, answer[0].Rdata) + } +} func TestDecodeDns_HeaderTooShort(t *testing.T) { decoded := []byte{183, 59}