[2.9] Fixes (#476)

[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 <pietro.dellamore@suse.com>
rancher · Feb 7, 2024 · 3bb70b7 · 3bb70b7
1 parent a182097
commit 3bb70b7
Show file tree

Hide file tree

Showing 3 changed files with 172 additions and 6 deletions.
diff --git a/api/server_test.go 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               = "<script>alert('xss')</script>"
+		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
@@ -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 &#x41;. 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
@@ -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