From a6a6cf5696088c32002953d36b75bdcc84f2399e Mon Sep 17 00:00:00 2001 From: Peter Matseykanets Date: Mon, 5 Feb 2024 10:46:41 -0500 Subject: [PATCH] [2.8] Fixes (#471) [v2.8.s3] Html escaping [2.8] Bump API-UI version --------- Co-authored-by: Kevin Joiner <10265309+KevinJoiner@users.noreply.github.com> Co-authored-by: pdellamore --- api/server_test.go | 137 +++++++++++++++++++++++++++++++++++++++++++++ api/writer/html.go | 30 +++++++++- urlbuilder/url.go | 11 ++-- 3 files changed, 172 insertions(+), 6 deletions(-) create mode 100644 api/server_test.go diff --git a/api/server_test.go b/api/server_test.go new file mode 100644 index 00000000..6dcc4e07 --- /dev/null +++ b/api/server_test.go @@ -0,0 +1,137 @@ +package api_test + +import ( + "fmt" + "net/http" + "net/http/httptest" + "net/url" + "strings" + "testing" + + "github.com/rancher/norman/api" + "github.com/rancher/norman/api/builtin" + "github.com/rancher/norman/api/writer" + "github.com/stretchr/testify/require" +) + +func TestServeHTMLEscaping(t *testing.T) { + const ( + defaultJS = "cattle.io" + defaultCSS = "cattle.io" + defaultAPIVersion = "v1.0.0" + xss = "" + alphaNumeric = "abcdefghijklmnopqrstuvABCDEFGHIJKLMNOPQRSTUV0123456789" + badChars = `~!@#$%^&*()_+-=[]\{}|;':",./<>?` + ) + xssUrl := url.URL{RawPath: xss} + + var escapedBadChars strings.Builder + for _, r := range badChars { + escapedBadChars.WriteString(fmt.Sprintf("&#x%X;", r)) + } + + t.Parallel() + tests := []struct { + name string + CSSURL string + JSURL string + APIUIVersion string + URL string + desiredContent string + undesiredContent string + }{ + { + name: "base case no xss", + CSSURL: defaultCSS, + JSURL: defaultJS, + APIUIVersion: defaultAPIVersion, + URL: "https://cattle.io/v3-publicHello", + desiredContent: "https://cattle.io/v3-publicHello", + }, + { + name: "JSS alpha-numeric", + CSSURL: defaultCSS, + JSURL: alphaNumeric, + APIUIVersion: defaultAPIVersion, + URL: "https://cattle.io/v3", + desiredContent: alphaNumeric, + }, + { + name: "JSS escaped non alpha-numeric", + CSSURL: defaultCSS, + JSURL: badChars, + APIUIVersion: defaultAPIVersion, + URL: "https://cattle.io/v3", + desiredContent: escapedBadChars.String(), + undesiredContent: badChars, + }, + { + name: "CSS alpha-numeric", + CSSURL: alphaNumeric, + JSURL: defaultJS, + APIUIVersion: defaultAPIVersion, + URL: "https://cattle.io/v3", + desiredContent: alphaNumeric, + }, + { + name: "CSS escaped non alpha-numeric", + CSSURL: badChars, + JSURL: defaultJS, + APIUIVersion: defaultAPIVersion, + URL: "https://cattle.io/v3", + desiredContent: escapedBadChars.String(), + undesiredContent: badChars, + }, + { + name: "api version alpha-numeric", + APIUIVersion: alphaNumeric, + URL: "https://cattle.io/v3", + desiredContent: alphaNumeric, + }, + { + name: "api version escaped non alpha-numeric", + APIUIVersion: badChars, + URL: "https://cattle.io/v3", + desiredContent: escapedBadChars.String(), + undesiredContent: badChars, + }, + { + name: "Link XSS", + URL: "https://cattle.io/v3" + xss, + undesiredContent: xss, + desiredContent: xssUrl.String(), + }, + } + for _, test := range tests { + tt := test + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + respStr, err := sendTestRequest(tt.URL, tt.CSSURL, tt.JSURL, tt.APIUIVersion) + require.NoError(t, err, "failed to create server") + require.Contains(t, respStr, tt.desiredContent, "expected content missing from server response") + if tt.undesiredContent != "" { + require.NotContains(t, respStr, tt.undesiredContent, "unexpected content found in server response") + } + }) + } +} + +func sendTestRequest(url, cssURL, jssURL, apiUIVersion string) (string, error) { + resp := httptest.NewRecorder() + req := httptest.NewRequest(http.MethodGet, url, nil) + // These header values are needed to get an HTML return document + req.Header.Set("Accept", "*/*") + req.Header.Set("User-agent", "Mozilla") + srv := api.NewAPIServer() + srv.CustomAPIUIResponseWriter(stringGetter(cssURL), stringGetter(jssURL), stringGetter(apiUIVersion)) + err := srv.AddSchemas(builtin.Schemas) + if err != nil { + return "", fmt.Errorf("failed to add builtin schemas: %w", err) + } + srv.ServeHTTP(resp, req) + return resp.Body.String(), nil +} + +func stringGetter(val string) writer.StringGetter { + return func() string { return val } +} diff --git a/api/writer/html.go b/api/writer/html.go index f1f952db..fb1e598c 100644 --- a/api/writer/html.go +++ b/api/writer/html.go @@ -2,6 +2,7 @@ package writer import ( "encoding/json" + "fmt" "strings" "github.com/rancher/norman/api/builtin" @@ -11,7 +12,7 @@ import ( const ( JSURL = "https://releases.rancher.com/api-ui/%API_UI_VERSION%/ui.min.js" CSSURL = "https://releases.rancher.com/api-ui/%API_UI_VERSION%/ui.min.css" - DefaultVersion = "1.1.10" + DefaultVersion = "1.1.11" ) var ( @@ -65,6 +66,11 @@ func (h *HTMLResponseWriter) Write(apiContext *types.APIContext, code int, obj i jsurl = strings.Replace(JSURL, "%API_UI_VERSION%", DefaultVersion, 1) cssurl = strings.Replace(CSSURL, "%API_UI_VERSION%", DefaultVersion, 1) } + + // jsurl and cssurl are added to the document as attributes not entities which requires special encoding. + jsurl, _ = encodeAttribute(jsurl) + cssurl, _ = encodeAttribute(cssurl) + headerString = strings.Replace(headerString, "%JSURL%", jsurl, 1) headerString = strings.Replace(headerString, "%CSSURL%", cssurl, 1) @@ -79,3 +85,25 @@ func jsonEncodeURL(str string) string { data, _ := json.Marshal(str) return string(data) } + +// encodeAttribute encodes all characters with the HTML Entity &#xHH; format, including spaces, where HH represents the hexadecimal value of the character in Unicode. +// For example, A becomes A. All alphanumeric characters (letters A to Z, a to z, and digits 0 to 9) remain unencoded. +// more info: https://cheatsheetseries.owasp.org/cheatsheets/Cross_Site_Scripting_Prevention_Cheat_Sheet.html#output-encoding-rules-summary +func encodeAttribute(raw string) (string, error) { + var builder strings.Builder + for _, r := range raw { + if ('A' <= r && r <= 'Z') || ('a' <= r && r <= 'z') || ('0' <= r && r <= '9') { + _, err := builder.WriteRune(r) + if err != nil { + return "", fmt.Errorf("failed to write: %w", err) + } + } else { + // encode non-alphanumeric rune to hex. + _, err := fmt.Fprintf(&builder, "&#x%X;", r) + if err != nil { + return "", fmt.Errorf("failed to write: %w", err) + } + } + } + return builder.String(), nil +} diff --git a/urlbuilder/url.go b/urlbuilder/url.go index 4678937f..46948f4b 100644 --- a/urlbuilder/url.go +++ b/urlbuilder/url.go @@ -2,7 +2,6 @@ package urlbuilder import ( "bytes" - "fmt" "net/http" "net/url" "strings" @@ -37,12 +36,14 @@ func New(r *http.Request, version types.APIVersion, schemas *types.Schemas) (typ } func ParseRequestURL(r *http.Request) string { - scheme := GetScheme(r) - host := GetHost(r, scheme) - return fmt.Sprintf("%s://%s%s%s", scheme, host, r.Header.Get(PrefixHeader), r.URL.Path) + var parsedURL url.URL + parsedURL.Scheme = GetScheme(r) + parsedURL.Host = GetHost(r) + parsedURL = *parsedURL.JoinPath(r.Header.Get(PrefixHeader), r.URL.Path) + return parsedURL.String() } -func GetHost(r *http.Request, scheme string) string { +func GetHost(r *http.Request) string { host := r.Header.Get(ForwardedAPIHostHeader) if host != "" { return host