From 334a78f1853ef14c96490acb0a579b38b69b3f7e Mon Sep 17 00:00:00 2001
From: Jakob Borg <jakob@kastelo.net>
Date: Mon, 2 May 2022 10:38:49 +0200
Subject: [PATCH] cmd/strelaysrv, cmd/strelaypoolsrv: Sanitize query strings
 (fixes #8314) (#8315)

Use the proper encoding function in the relay server when constructing
the URL. In the pool server, parse and re-encode the query values to
sanitize whatever the client sent.
---
 cmd/strelaypoolsrv/main.go      |  5 +++++
 cmd/strelaypoolsrv/main_test.go | 27 +++++++++++++++++++++++++++
 cmd/strelaysrv/main.go          | 21 ++++++++++++++++++++-
 3 files changed, 52 insertions(+), 1 deletion(-)

diff --git a/cmd/strelaypoolsrv/main.go b/cmd/strelaypoolsrv/main.go
index a0364a81c..be54fd3b9 100644
--- a/cmd/strelaypoolsrv/main.go
+++ b/cmd/strelaypoolsrv/main.go
@@ -368,6 +368,11 @@ func handlePostRequest(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
+	// Canonicalize the URL. In particular, parse and re-encode the query
+	// string so that it's guaranteed to be valid.
+	uri.RawQuery = uri.Query().Encode()
+	newRelay.URL = uri.String()
+
 	if relayCert != nil {
 		advertisedId := uri.Query().Get("id")
 		idFromCert := protocol.NewDeviceID(relayCert.Raw).String()
diff --git a/cmd/strelaypoolsrv/main_test.go b/cmd/strelaypoolsrv/main_test.go
index 6b5732f12..db7584655 100644
--- a/cmd/strelaypoolsrv/main_test.go
+++ b/cmd/strelaypoolsrv/main_test.go
@@ -11,6 +11,7 @@ import (
 	"encoding/json"
 	"fmt"
 	"net/http/httptest"
+	"net/url"
 	"strings"
 	"sync"
 	"testing"
@@ -65,3 +66,29 @@ func TestHandleGetRequest(t *testing.T) {
 		}
 	}
 }
+
+func TestCanonicalizeQueryValues(t *testing.T) {
+	// This just demonstrates and validates the uri.Parse/String stuff in
+	// regards to query strings.
+
+	in := "http://example.com/?some weird= query^value"
+	exp := "http://example.com/?some+weird=+query%5Evalue"
+
+	uri, err := url.Parse(in)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	str := uri.String()
+	if str != in {
+		// Just re-encoding the URL doesn't sanitize the query string.
+		t.Errorf("expected %q, got %q", in, str)
+	}
+
+	uri.RawQuery = uri.Query().Encode()
+	str = uri.String()
+	if str != exp {
+		// The query string is now in correct format.
+		t.Errorf("expected %q, got %q", exp, str)
+	}
+}
diff --git a/cmd/strelaysrv/main.go b/cmd/strelaysrv/main.go
index 507b3e6c0..4757f90b0 100644
--- a/cmd/strelaysrv/main.go
+++ b/cmd/strelaysrv/main.go
@@ -230,12 +230,31 @@ func main() {
 		go statusService(statusAddr)
 	}
 
-	uri, err := url.Parse(fmt.Sprintf("relay://%s/?id=%s&pingInterval=%s&networkTimeout=%s&sessionLimitBps=%d&globalLimitBps=%d&statusAddr=%s&providedBy=%s", mapping.Address(), id, pingInterval, networkTimeout, sessionLimitBps, globalLimitBps, statusAddr, providedBy))
+	uri, err := url.Parse(fmt.Sprintf("relay://%s/", mapping.Address()))
 	if err != nil {
 		log.Fatalln("Failed to construct URI", err)
 		return
 	}
 
+	// Add properly encoded query string parameters to URL.
+	query := make(url.Values)
+	query.Set("id", id.String())
+	query.Set("pingInterval", pingInterval.String())
+	query.Set("networkTimeout", networkTimeout.String())
+	if sessionLimitBps > 0 {
+		query.Set("sessionLimitBps", fmt.Sprint(sessionLimitBps))
+	}
+	if globalLimitBps > 0 {
+		query.Set("globalLimitBps", fmt.Sprint(globalLimitBps))
+	}
+	if statusAddr != "" {
+		query.Set("statusAddr", statusAddr)
+	}
+	if providedBy != "" {
+		query.Set("providedBy", providedBy)
+	}
+	uri.RawQuery = query.Encode()
+
 	log.Println("URI:", uri.String())
 
 	if poolAddrs == defaultPoolAddrs {