frontend: fix XSS (#57) (#58)

frontend/assets/templates/bgpmap.tpl

@@ -6,7 +6,7 @@
 <script src="/static/jsdelivr/npm/viz.js@2.1.2/lite.render.js" crossorigin="anonymous"></script>
 <script>
   var viz = new Viz();
-  viz.renderSVGElement(`{{ .Result }}`)
+  viz.renderSVGElement(atob({{ .Result }}))
   .then(element => {
     document.getElementById("bgpmap").appendChild(element);
   })

frontend/assets/templates/page.tpl

@@ -37,8 +37,8 @@
 					href="{{ .AllServersURLCustom }}"> {{ .AllServerTitle }} </a>
 				{{ end }}
 			</li>
-			{{ $length := len .ServersEscaped }} 
+			{{ $length := len .Servers }} 
-			{{ range $k, $v := .ServersEscaped }}
+			{{ range $k, $v := .Servers }}
 			<li class="nav-item">
 				{{ if gt $length 1 }}
 				<a class="nav-link{{ if eq $server $v }} active{{ end }}"

frontend/render.go

@@ -4,12 +4,11 @@ import (
 	"bytes"
 	"errors"
 	"fmt"
+	"html/template"
 	"net/http"
-	"net/url"
 	"regexp"
 	"sort"
 	"strings"
-	"text/template"
 )
 
 // static options map
@@ -38,7 +37,7 @@ var summaryStateMap = map[string]string{
 }
 
 // render the page template
-func renderPageTemplate(w http.ResponseWriter, r *http.Request, title string, content string) {
+func renderPageTemplate(w http.ResponseWriter, r *http.Request, title string, content template.HTML) {
 	path := r.URL.Path[1:]
 	split := strings.SplitN(path, "/", 3)
 
@@ -48,32 +47,26 @@ func renderPageTemplate(w http.ResponseWriter, r *http.Request, title string, co
 	// Use a default URL if the request URL is too short
 	// The URL is for return to summary page
 	if len(split) < 2 {
-		path = "summary/" + url.PathEscape(strings.Join(setting.servers, "+")) + "/"
+		path = "summary/" + strings.Join(setting.servers, "+") + "/"
 	} else if len(split) == 2 {
 		path += "/"
 	}
 
 	split = strings.SplitN(path, "/", 3)
 
-	serversEscaped := make([]string, len(setting.servers))
-	for i, v := range setting.servers {
-		serversEscaped[i] = url.PathEscape(v)
-	}
-
 	args := TemplatePage{
 		Options:              optionsMap,
 		Servers:              setting.servers,
-		ServersEscaped:       serversEscaped,
 		ServersDisplay:       setting.serversDisplay,
-		AllServersLinkActive: strings.ToLower(split[1]) == strings.ToLower(strings.Join(setting.servers, "+")),
+		AllServersLinkActive: strings.EqualFold(split[1], strings.Join(setting.servers, "+")),
-		AllServersURL:        url.PathEscape(strings.Join(setting.servers, "+")),
+		AllServersURL:        strings.Join(setting.servers, "+"),
 		AllServerTitle:       setting.navBarAllServer,
 		AllServersURLCustom:  setting.navBarAllURL,
 		IsWhois:              isWhois,
 		WhoisTarget:          whoisTarget,
 
 		URLOption:  strings.ToLower(split[0]),
-		URLServer:  url.PathEscape(strings.ToLower(split[1])),
+		URLServer:  strings.ToLower(split[1]),
 		URLCommand: split[2],
 		Title:      setting.titleBrand + title,
 		Brand:      setting.navBarBrand,
@@ -91,7 +84,7 @@ func renderPageTemplate(w http.ResponseWriter, r *http.Request, title string, co
 
 // Write the given text to http response, and add whois links for
 // ASNs and IP addresses
-func smartFormatter(s string) string {
+func smartFormatter(s string) template.HTML {
 	var result string
 	result += "<pre>"
 	s = template.HTMLEscapeString(s)
@@ -108,7 +101,7 @@ func smartFormatter(s string) string {
 		result += lineFormatted + "\n"
 	}
 	result += "</pre>"
-	return result
+	return template.HTML(result)
 }
 
 // Parse bird show protocols result
@@ -200,11 +193,11 @@ func summaryParse(data string, serverName string) (TemplateSummary, error) {
 }
 
 // Output a table for the summary page
-func summaryTable(data string, serverName string) string {
+func summaryTable(data string, serverName string) template.HTML {
 	result, err := summaryParse(data, serverName)
 
 	if err != nil {
-		return "<pre>" + template.HTMLEscapeString(err.Error()) + "</pre>"
+		return template.HTML("<pre>" + template.HTMLEscapeString(err.Error()) + "</pre>")
 	}
 
 	// render the summary template
@@ -215,5 +208,5 @@ func summaryTable(data string, serverName string) string {
 		fmt.Println("Error rendering summary:", err.Error())
 	}
 
-	return buffer.String()
+	return template.HTML(buffer.String())
 }

frontend/render_test.go

@@ -1,9 +1,9 @@
 package main
 
 import (
+	"html/template"
 	"io/ioutil"
 	"net/http/httptest"
-	"net/url"
 	"strings"
 	"testing"
 )
@@ -25,7 +25,7 @@ func TestRenderPageTemplate(t *testing.T) {
 
 	r := httptest.NewRequest("GET", "/route/alpha/192.168.0.1/", nil)
 	w := httptest.NewRecorder()
-	renderPageTemplate(w, r, title, content)
+	renderPageTemplate(w, r, title, template.HTML(content))
 
 	resultBytes, _ := ioutil.ReadAll(w.Result().Body)
 	result := string(resultBytes)
@@ -43,7 +43,27 @@ func TestRenderPageTemplateXSS(t *testing.T) {
 
 	evil := "<script>alert('evil');</script>"
 
-	r := httptest.NewRequest("GET", "/whois/"+url.PathEscape(evil), nil)
+	r := httptest.NewRequest("GET", "/whois/"+evil, nil)
+	w := httptest.NewRecorder()
+
+	// renderPageTemplate doesn't escape content, filter is done beforehand
+	renderPageTemplate(w, r, evil, "Test Content")
+
+	resultBytes, _ := ioutil.ReadAll(w.Result().Body)
+	result := string(resultBytes)
+
+	if strings.Contains(result, evil) {
+		t.Errorf("XSS injection succeeded: %s", result)
+	}
+}
+
+// https://github.com/xddxdd/bird-lg-go/issues/57
+func TestRenderPageTemplateXSS_2(t *testing.T) {
+	initSettings()
+
+	evil := "<script>alert('evil');</script>"
+
+	r := httptest.NewRequest("GET", "/generic/dummy_server/"+evil, nil)
 	w := httptest.NewRecorder()
 
 	// renderPageTemplate doesn't escape content, filter is done beforehand
@@ -59,7 +79,7 @@ func TestRenderPageTemplateXSS(t *testing.T) {
 
 func TestSmartFormatterXSS(t *testing.T) {
 	evil := "<script>alert('evil');</script>"
-	result := smartFormatter(evil)
+	result := string(smartFormatter(evil))
 
 	if strings.Contains(result, evil) {
 		t.Errorf("XSS injection succeeded: %s", result)
@@ -71,7 +91,7 @@ func TestSummaryTableXSS(t *testing.T) {
 	evilData := `Name       Proto      Table      State  Since         Info
 ` + evil + ` ` + evil + `       ---        up     2021-01-04 17:21:44  ` + evil
 
-	result := summaryTable(evilData, evil)
+	result := string(summaryTable(evilData, evil))
 
 	if strings.Contains(result, evil) {
 		t.Errorf("XSS injection succeeded: %s", result)
@@ -91,7 +111,7 @@ kernel2    Kernel     master4    up     2021-08-27
 direct1    Direct     ---        up     2021-08-27
 int_babel  Babel      ---        up     2021-08-27    `
 
-	result := summaryTable(data, "testserver")
+	result := string(summaryTable(data, "testserver"))
 	expectedInclude := []string{"static1", "static2", "int_babel", "direct1"}
 	expectedExclude := []string{"device1", "kernel1", "kernel2"}
 
@@ -124,7 +144,7 @@ kernel2    Kernel     master4    up     2021-08-27
 direct1    Direct     ---        up     2021-08-27
 int_babel  Babel      ---        up     2021-08-27    `
 
-	result := summaryTable(data, "testserver")
+	result := string(summaryTable(data, "testserver"))
 	expectedInclude := []string{"device1", "kernel1", "kernel2", "direct1", "int_babel"}
 	expectedExclude := []string{"static1", "static2"}
 

frontend/template.go

@@ -2,8 +2,8 @@ package main
 
 import (
 	"embed"
+	"html/template"
 	"strings"
-	"text/template"
 )
 
 // import templates and other assets
@@ -19,7 +19,6 @@ type TemplatePage struct {
 	// Global options
 	Options        map[string]string
 	Servers        []string
-	ServersEscaped []string
 	ServersDisplay []string
 
 	// Parameters related to current request
@@ -40,7 +39,7 @@ type TemplatePage struct {
 	Title    string
 	Brand    string
 	BrandURL string
-	Content  string
+	Content  template.HTML
 }
 
 // summary
@@ -74,7 +73,7 @@ type TemplateSummary struct {
 // whois
 type TemplateWhois struct {
 	Target string
-	Result string
+	Result template.HTML
 }
 
 // bgpmap
@@ -88,7 +87,7 @@ type TemplateBGPmap struct {
 type TemplateBird struct {
 	ServerName string
 	Target     string
-	Result     string
+	Result     template.HTML
 }
 
 // global variable to hold the templates

frontend/webserver.go

@@ -2,8 +2,10 @@ package main
 
 import (
 	"bytes"
+	"encoding/base64"
 	"fmt"
 	"html"
+	"html/template"
 	"io/fs"
 	"net"
 	"net/http"
@@ -56,7 +58,7 @@ func webHandlerWhois(w http.ResponseWriter, r *http.Request) {
 	renderPageTemplate(
 		w, r,
 		" - whois "+html.EscapeString(target),
-		buffer.String(),
+		template.HTML(buffer.String()),
 	)
 }
 
@@ -89,7 +91,7 @@ func webBackendCommunicator(endpoint string, command string) func(w http.Respons
 		var content string
 		for i, response := range responses {
 
-			var result string
+			var result template.HTML
 			if (endpoint == "bird") && backendCommand == "show protocols" && len(response) > 4 && strings.ToLower(response[0:4]) == "name" {
 				result = summaryTable(response, servers[i])
 			} else {
@@ -124,7 +126,7 @@ func webBackendCommunicator(endpoint string, command string) func(w http.Respons
 		renderPageTemplate(
 			w, r,
 			" - "+endpoint+" "+backendCommand,
-			content,
+			template.HTML(content),
 		)
 	}
 }
@@ -154,11 +156,15 @@ func webHandlerBGPMap(endpoint string, command string) func(w http.ResponseWrite
 		var servers []string = strings.Split(split[1], "+")
 		var responses []string = batchRequest(servers, endpoint, backendCommand)
 
+		// encode result with base64 to prevent xss
+		result := birdRouteToGraphviz(servers, responses, urlCommands)
+		result = base64.StdEncoding.EncodeToString([]byte(result))
+
 		// render the bgpmap result template
 		args := TemplateBGPmap{
 			Servers: servers,
 			Target:  backendCommand,
-			Result:  birdRouteToGraphviz(servers, responses, urlCommands),
+			Result:  result,
 		}
 
 		tmpl := TemplateLibrary["bgpmap"]
@@ -171,7 +177,7 @@ func webHandlerBGPMap(endpoint string, command string) func(w http.ResponseWrite
 		renderPageTemplate(
 			w, r,
 			" - "+html.EscapeString(endpoint+" "+backendCommand),
-			buffer.String(),
+			template.HTML(buffer.String()),
 		)
 	}
 }