From 5d80a6e21e415262a4753760032f4d8fdc8d1216 Mon Sep 17 00:00:00 2001
From: Miek Gieben <miek@miek.nl>
Date: Wed, 19 May 2021 19:38:37 +0200
Subject: [PATCH] Fix obsure crash in Corefile parsing (#4637)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

This was found by fuzzing.

We need to make this a fully qualified domain name to catch all errors
in dnsserver/register.go and not later when plugin.Normalize() is called again on these
strings, with the prime difference being that the domain name is fully
qualified. This was found by fuzzing where "ȶ" is deemed OK, but "ȶ." is
not (might be a bug in miekg/dns actually). But here we were checking ȶ,
which is OK, and later we barf in ȶ. leading to "index out of range".

Added a tests and check manually if it would crash with the current code
(yes), and fail with an error in this PR (yes).

Signed-off-by: Miek Gieben <miek@miek.nl>
---
 core/dnsserver/register.go | 11 +++++++++++
 plugin/normalize.go        |  6 +++++-
 test/corefile_test.go      | 17 +++++++++++++++++
 3 files changed, 33 insertions(+), 1 deletion(-)
 create mode 100644 test/corefile_test.go

diff --git a/core/dnsserver/register.go b/core/dnsserver/register.go
index 1eb457b8e..a407c8938 100644
--- a/core/dnsserver/register.go
+++ b/core/dnsserver/register.go
@@ -66,6 +66,17 @@ func (h *dnsContext) InspectServerBlocks(sourceFile string, serverBlocks []caddy
 		for ik, k := range s.Keys {
 			trans, k1 := parse.Transport(k) // get rid of any dns:// or other scheme.
 			hosts, port, err := plugin.SplitHostPort(k1)
+			// We need to make this a fully qualified domain name to catch all errors here and not later when
+			// plugin.Normalize is called again on these strings, with the prime difference being that the domain
+			// name is fully qualified. This was found by fuzzing where "ȶ" is deemed OK, but "ȶ." is not (might be a
+			// bug in miekg/dns actually). But here we were checking ȶ, which is OK, and later we barf in ȶ. leading to
+			// "index out of range".
+			for ih := range hosts {
+				_, _, err := plugin.SplitHostPort(dns.Fqdn(hosts[ih]))
+				if err != nil {
+					return nil, err
+				}
+			}
 			if err != nil {
 				return nil, err
 			}
diff --git a/plugin/normalize.go b/plugin/normalize.go
index 96ec59c76..10a60a806 100644
--- a/plugin/normalize.go
+++ b/plugin/normalize.go
@@ -132,7 +132,11 @@ func OriginsFromArgsOrServerBlock(args, serverblock []string) []string {
 	}
 	s := []string{}
 	for i := range args {
-		s = append(s, Host(args[i]).Normalize()...)
+		sx := Host(args[i]).Normalize()
+		if len(sx) == 0 {
+			continue // silently ignores errors.
+		}
+		s = append(s, sx...)
 	}
 
 	return s
diff --git a/test/corefile_test.go b/test/corefile_test.go
new file mode 100644
index 000000000..1f08ab2f1
--- /dev/null
+++ b/test/corefile_test.go
@@ -0,0 +1,17 @@
+package test
+
+import (
+	"testing"
+)
+
+func TestCorefile1(t *testing.T) {
+	corefile := `ȶ
+acl
+`
+	// this crashed, now it should return an error.
+	i, _, _, err := CoreDNSServerAndPorts(corefile)
+	if err == nil {
+		defer i.Stop()
+		t.Fatalf("Expected an error got none")
+	}
+}