From 982326a6786b835b81642de80d9373d1bb6c3ae3 Mon Sep 17 00:00:00 2001 From: Yuhui Xu Date: Fri, 5 Aug 2022 21:59:18 -0400 Subject: [PATCH] frontend: fix XSS (#57) (#58) --- frontend/assets/templates/bgpmap.tpl | 2 +- frontend/assets/templates/page.tpl | 4 ++-- frontend/render.go | 29 +++++++++--------------- frontend/render_test.go | 34 ++++++++++++++++++++++------ frontend/template.go | 9 ++++---- frontend/webserver.go | 16 +++++++++---- 6 files changed, 56 insertions(+), 38 deletions(-) diff --git a/frontend/assets/templates/bgpmap.tpl b/frontend/assets/templates/bgpmap.tpl index 4c67ccc..8872a60 100644 --- a/frontend/assets/templates/bgpmap.tpl +++ b/frontend/assets/templates/bgpmap.tpl @@ -6,7 +6,7 @@ " - 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 := "" + + 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 := "" - 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"} diff --git a/frontend/template.go b/frontend/template.go index 4f0ec10..2f993f6 100644 --- a/frontend/template.go +++ b/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 diff --git a/frontend/webserver.go b/frontend/webserver.go index 9bd589a..e01a270 100644 --- a/frontend/webserver.go +++ b/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()), ) } }