From 97319a19704acda4456f8b1951479fbf137a557a Mon Sep 17 00:00:00 2001
From: Brad Fitzpatrick <bradfitz@tailscale.com>
Date: Wed, 16 Nov 2022 07:33:31 -0800
Subject: [PATCH] control/controlclient: filter PopBrowserURL values to https
 schemes

No need for http://, etc. In case a control server sends a bogus value
and GUIs don't also validate.

Updates tailscale/corp#7948

Change-Id: I0b7dd86aa396bdabd88f0c4fe51831fb2ec4175a
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
---
 control/controlclient/direct.go | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/control/controlclient/direct.go b/control/controlclient/direct.go
index b889a8fe3..4ad0968ce 100644
--- a/control/controlclient/direct.go
+++ b/control/controlclient/direct.go
@@ -999,6 +999,9 @@ func (c *Direct) sendMapRequest(ctx context.Context, maxPolls int, readOnly bool
 			metricMapResponsePings.Add(1)
 			go c.answerPing(pr)
 		}
+		if !c.validPopBrowserURL(resp.PopBrowserURL) {
+			resp.PopBrowserURL = ""
+		}
 		if u := resp.PopBrowserURL; u != "" && u != sess.lastPopBrowserURL {
 			sess.lastPopBrowserURL = u
 			if c.popBrowser != nil {
@@ -1702,6 +1705,28 @@ func (c *Direct) ReportHealthChange(sys health.Subsystem, sysErr error) {
 	res.Body.Close()
 }
 
+// validPopBrowserURL reports whether urlStr is a valid value for a
+// control server to send in a MapResponse.PopUpBrowserURL field.
+func (c *Direct) validPopBrowserURL(urlStr string) bool {
+	if urlStr == "" {
+		// Common case.
+		return true
+	}
+	u, err := url.Parse(urlStr)
+	if err != nil {
+		return false
+	}
+	switch u.Scheme {
+	case "https":
+		return true
+	case "http":
+		// If the control server is using plain HTTP (likely a dev server),
+		// then permit http://.
+		return strings.HasPrefix(c.serverURL, "http://")
+	}
+	return false
+}
+
 var (
 	metricMapRequestsActive = clientmetric.NewGauge("controlclient_map_requests_active")