From 65a1dfb3db2ad85763f26417f8699486bc253369 Mon Sep 17 00:00:00 2001 From: Chris Golden Date: Sat, 6 Aug 2022 22:35:06 -0700 Subject: [PATCH] Fix two panics identified by fuzz testing This commit adds additional bounds checking, one for DNSResourceRecord and another for DNSQuestion. The DNSResourceRecord panic was observed in production and the second panic was caught by fuzzing. Add a fuzz test with seeds to reproduce these two panics. --- layers/dns.go | 8 ++++++++ layers/dns_test.go | 7 +++++++ ...ce7b719228870c23869cf21bf8829d7160c82da88f80aeff2d861c | 2 ++ ...21f20980b59f64e1617d6b334b152a90b141a7407c3c8fa4837a31 | 2 ++ ...cf68c2129abd63d4c7cfb1979c489ad9bad6898510a4d4759bb85f | 2 ++ ...2057b406a06ec4f04065f6d0dda6b2b362c0a89192c1933e927adf | 2 ++ 6 files changed, 23 insertions(+) create mode 100644 layers/testdata/fuzz/FuzzDecodeFromBytes/27d23183d8ce7b719228870c23869cf21bf8829d7160c82da88f80aeff2d861c create mode 100644 layers/testdata/fuzz/FuzzDecodeFromBytes/3b53f220d321f20980b59f64e1617d6b334b152a90b141a7407c3c8fa4837a31 create mode 100644 layers/testdata/fuzz/FuzzDecodeFromBytes/f539b7a397cf68c2129abd63d4c7cfb1979c489ad9bad6898510a4d4759bb85f create mode 100644 layers/testdata/fuzz/FuzzDecodeFromBytes/fe20300d6b2057b406a06ec4f04065f6d0dda6b2b362c0a89192c1933e927adf diff --git a/layers/dns.go b/layers/dns.go index b639dec15..21b6de6e5 100644 --- a/layers/dns.go +++ b/layers/dns.go @@ -639,6 +639,10 @@ func (q *DNSQuestion) decode(data []byte, offset int, df gopacket.DecodeFeedback return 0, err } + if len(data) < endq+4 { + return 0, errors.New("DNS question too small") + } + q.Name = name q.Type = DNSType(binary.BigEndian.Uint16(data[endq : endq+2])) q.Class = DNSClass(binary.BigEndian.Uint16(data[endq+2 : endq+4])) @@ -709,6 +713,10 @@ func (rr *DNSResourceRecord) decode(data []byte, offset int, df gopacket.DecodeF return 0, err } + if len(data) < endq+10 { + return 0, errors.New("DNS record too small") + } + rr.Name = name rr.Type = DNSType(binary.BigEndian.Uint16(data[endq : endq+2])) rr.Class = DNSClass(binary.BigEndian.Uint16(data[endq+2 : endq+4])) diff --git a/layers/dns_test.go b/layers/dns_test.go index 8c16766df..5d7cbad32 100644 --- a/layers/dns_test.go +++ b/layers/dns_test.go @@ -15,6 +15,13 @@ import ( "github.com/google/gopacket" ) +func FuzzDecodeFromBytes(f *testing.F) { + f.Fuzz(func(t *testing.T, bytes []byte) { + dns := DNS{} + dns.DecodeFromBytes(bytes, gopacket.NilDecodeFeedback) + }) +} + // it have a layer like that: // name: xxx.com // type: CNAME diff --git a/layers/testdata/fuzz/FuzzDecodeFromBytes/27d23183d8ce7b719228870c23869cf21bf8829d7160c82da88f80aeff2d861c b/layers/testdata/fuzz/FuzzDecodeFromBytes/27d23183d8ce7b719228870c23869cf21bf8829d7160c82da88f80aeff2d861c new file mode 100644 index 000000000..9752921be --- /dev/null +++ b/layers/testdata/fuzz/FuzzDecodeFromBytes/27d23183d8ce7b719228870c23869cf21bf8829d7160c82da88f80aeff2d861c @@ -0,0 +1,2 @@ +go test fuzz v1 +[]byte("0000000\x10\x10\x00\x01\x01\x01\x01\x01\x01\x00") diff --git a/layers/testdata/fuzz/FuzzDecodeFromBytes/3b53f220d321f20980b59f64e1617d6b334b152a90b141a7407c3c8fa4837a31 b/layers/testdata/fuzz/FuzzDecodeFromBytes/3b53f220d321f20980b59f64e1617d6b334b152a90b141a7407c3c8fa4837a31 new file mode 100644 index 000000000..80df1f957 --- /dev/null +++ b/layers/testdata/fuzz/FuzzDecodeFromBytes/3b53f220d321f20980b59f64e1617d6b334b152a90b141a7407c3c8fa4837a31 @@ -0,0 +1,2 @@ +go test fuzz v1 +[]byte("000000000000\x00") diff --git a/layers/testdata/fuzz/FuzzDecodeFromBytes/f539b7a397cf68c2129abd63d4c7cfb1979c489ad9bad6898510a4d4759bb85f b/layers/testdata/fuzz/FuzzDecodeFromBytes/f539b7a397cf68c2129abd63d4c7cfb1979c489ad9bad6898510a4d4759bb85f new file mode 100644 index 000000000..4c717ec9d --- /dev/null +++ b/layers/testdata/fuzz/FuzzDecodeFromBytes/f539b7a397cf68c2129abd63d4c7cfb1979c489ad9bad6898510a4d4759bb85f @@ -0,0 +1,2 @@ +go test fuzz v1 +[]byte("01000\x10\x10\xdfd\x01\x01\x01\x00d\x01\x01\x01\x00") diff --git a/layers/testdata/fuzz/FuzzDecodeFromBytes/fe20300d6b2057b406a06ec4f04065f6d0dda6b2b362c0a89192c1933e927adf b/layers/testdata/fuzz/FuzzDecodeFromBytes/fe20300d6b2057b406a06ec4f04065f6d0dda6b2b362c0a89192c1933e927adf new file mode 100644 index 000000000..e9da5747d --- /dev/null +++ b/layers/testdata/fuzz/FuzzDecodeFromBytes/fe20300d6b2057b406a06ec4f04065f6d0dda6b2b362c0a89192c1933e927adf @@ -0,0 +1,2 @@ +go test fuzz v1 +[]byte("0000\x00\x00000000\x010\x000")