fix: harden request handling and input parsing in hello and diagnostics

cmd/cloudflared/tunnel/login.go

@@ -5,7 +5,6 @@ import (
 	"net/url"
 	"os"
 	"path/filepath"
-	"syscall"
 
 	homedir "github.com/mitchellh/go-homedir"
 	"github.com/pkg/errors"
@@ -149,7 +148,7 @@ func checkForExistingCert() (string, bool, error) {
 	if err == nil && fileInfo.Size() > 0 {
 		return path, true, nil
 	}
-	if err != nil && err.(*os.PathError).Err != syscall.ENOENT {
+	if err != nil && !os.IsNotExist(err) {
 		return path, false, err
 	}
 

diagnostic/network/collector.go

@@ -8,7 +8,10 @@ import (
 
 const MicrosecondsFactor = 1000.0
 
-var ErrEmptyDomain = errors.New("domain must not be empty")
+var (
+	ErrEmptyDomain  = errors.New("domain must not be empty")
+	ErrEmptyHopLine = errors.New("hop line must not be empty")
+)
 
 // For now only support ICMP is provided.
 type IPVersion int

diagnostic/network/collector_unix.go

@@ -48,6 +48,10 @@ func DecodeLine(text string) (*Hop, error) {
 		}
 	}
 
+	if len(parts) == 0 {
+		return nil, ErrEmptyHopLine
+	}
+
 	index, err := strconv.ParseUint(parts[0], 10, 8)
 	if err != nil {
 		return nil, fmt.Errorf("couldn't parse index from timeout hop: %w", err)

diagnostic/network/collector_unix_test.go

@@ -159,6 +159,32 @@ someletters 8.8.8.8 8.8.8.9 abc ms 0.456 ms 0.789 ms`,
 				),
 			},
 		},
+		{
+			"line without a hop index is skipped",
+			`1  172.68.101.121 (172.68.101.121)  12.874 ms  15.517 ms  15.311 ms
+* * *
+2  172.68.101.121 (172.68.101.121)  12.874 ms  15.517 ms  15.311 ms `,
+			[]*diagnostic.Hop{
+				diagnostic.NewHop(
+					uint8(1),
+					"172.68.101.121 (172.68.101.121)",
+					[]time.Duration{
+						time.Duration(12874),
+						time.Duration(15517),
+						time.Duration(15311),
+					},
+				),
+				diagnostic.NewHop(
+					uint8(2),
+					"172.68.101.121 (172.68.101.121)",
+					[]time.Duration{
+						time.Duration(12874),
+						time.Duration(15517),
+						time.Duration(15311),
+					},
+				),
+			},
+		},
 	}
 
 	for _, test := range tests {

diagnostic/network/collector_windows.go

@@ -47,6 +47,10 @@ func DecodeLine(text string) (*Hop, error) {
 		}
 	}
 
+	if len(parts) == 0 {
+		return nil, ErrEmptyHopLine
+	}
+
 	index, err := strconv.ParseUint(parts[0], 10, 8)
 	if err != nil {
 		return nil, fmt.Errorf("couldn't parse index from timeout hop: %w", err)

diagnostic/network/collector_windows_test.go

@@ -196,6 +196,32 @@ someletters abc ms 0.456 ms 0.789 ms 8.8.8.8 8.8.8.9`,
 				),
 			},
 		},
+		{
+			"line without a hop index is skipped",
+			`1  12.874 ms  15.517 ms  15.311 ms 172.68.101.121
+* * *
+2  12.874 ms  15.517 ms  15.311 ms 172.68.101.121`,
+			[]*diagnostic.Hop{
+				diagnostic.NewHop(
+					uint8(1),
+					"172.68.101.121",
+					[]time.Duration{
+						time.Duration(12874),
+						time.Duration(15517),
+						time.Duration(15311),
+					},
+				),
+				diagnostic.NewHop(
+					uint8(2),
+					"172.68.101.121",
+					[]time.Duration{
+						time.Duration(12874),
+						time.Duration(15517),
+						time.Duration(15311),
+					},
+				),
+			},
+		},
 	}
 
 	for _, test := range tests {

hello/hello.go

@@ -19,11 +19,12 @@ import (
 )
 
 const (
-	UptimeRoute    = "/uptime"
-	WSRoute        = "/ws"
-	SSERoute       = "/sse"
-	HealthRoute    = "/_health"
-	defaultSSEFreq = time.Second * 10
+	UptimeRoute              = "/uptime"
+	WSRoute                  = "/ws"
+	SSERoute                 = "/sse"
+	HealthRoute              = "/_health"
+	defaultSSEFreq           = time.Second * 10
+	defaultReadHeaderTimeout = time.Second * 10
 )
 
 type templateData struct {
@@ -117,7 +118,11 @@ func StartHelloWorldServer(log *zerolog.Logger, listener net.Listener, shutdownC
 	muxer.HandleFunc(SSERoute, sseHandler(log))
 	muxer.HandleFunc(HealthRoute, healthHandler())
 	muxer.HandleFunc("/", rootHandler(serverName))
-	httpServer := &http.Server{Addr: listener.Addr().String(), Handler: muxer}
+	httpServer := &http.Server{
+		Addr:              listener.Addr().String(),
+		Handler:           muxer,
+		ReadHeaderTimeout: defaultReadHeaderTimeout,
+	}
 	go func() {
 		<-shutdownC
 		_ = httpServer.Close()
@@ -200,7 +205,7 @@ func sseHandler(log *zerolog.Logger) http.HandlerFunc {
 		freq := defaultSSEFreq
 		if requestedFreq := r.URL.Query()["freq"]; len(requestedFreq) > 0 {
 			parsedFreq, err := time.ParseDuration(requestedFreq[0])
-			if err == nil {
+			if err == nil && parsedFreq > 0 {
 				freq = parsedFreq
 			}
 		}