diff --git a/internal/htmlsanitize/htmlsanitize.go b/internal/htmlsanitize/htmlsanitize.go new file mode 100644 --- /dev/null +++ b/internal/htmlsanitize/htmlsanitize.go @@ -0,0 +1,100 @@ +package htmlsanitize + +import ( + "bytes" + "encoding/json" + "html" + "io" + "mime" + "strings" +) + +// String escapes text before it is returned to browser-facing management clients. +func String(value string) string { + return html.EscapeString(value) +} + +// Strings escapes each string in values while preserving order. +func Strings(values []string) []string { + out := make([]string, 0, len(values)) + for _, value := range values { + out = append(out, String(value)) + } + return out +} + +// JSONBody escapes all string values in a JSON document. +func JSONBody(body []byte) ([]byte, bool) { + trimmed := bytes.TrimSpace(body) + if len(trimmed) == 0 { + return body, false + } + + decoder := json.NewDecoder(bytes.NewReader(trimmed)) + decoder.UseNumber() + var value any + if errDecode := decoder.Decode(&value); errDecode != nil { + return body, false + } + var extra any + if errExtra := decoder.Decode(&extra); errExtra != io.EOF { + return body, false + } + + var buffer bytes.Buffer + encoder := json.NewEncoder(&buffer) + encoder.SetEscapeHTML(false) + if errEncode := encoder.Encode(JSONValue(value)); errEncode != nil { + return body, false + } + return bytes.TrimSuffix(buffer.Bytes(), []byte("\n")), true +} + +// JSONBodyIfLikely escapes JSON bodies when the content type or body shape indicates JSON. +func JSONBodyIfLikely(body []byte, contentType string) ([]byte, bool) { + if IsJSONContentType(contentType) || LooksLikeJSON(body) { + return JSONBody(body) + } + return body, false +} + +// JSONValue recursively escapes string values in JSON-compatible data. +func JSONValue(value any) any { + switch typed := value.(type) { + case string: + return String(typed) + case []any: + out := make([]any, len(typed)) + for index, item := range typed { + out[index] = JSONValue(item) + } + return out + case map[string]any: + out := make(map[string]any, len(typed)) + for key, item := range typed { + out[key] = JSONValue(item) + } + return out + default: + return value + } +} + +// IsJSONContentType reports whether contentType is application/json or a +json type. +func IsJSONContentType(contentType string) bool { + mediaType, _, errParse := mime.ParseMediaType(strings.TrimSpace(contentType)) + if errParse != nil { + mediaType = strings.TrimSpace(contentType) + } + mediaType = strings.ToLower(mediaType) + return mediaType == "application/json" || strings.HasSuffix(mediaType, "+json") +} + +// LooksLikeJSON reports whether body starts with an object or array JSON marker. +func LooksLikeJSON(body []byte) bool { + trimmed := bytes.TrimSpace(body) + if len(trimmed) == 0 { + return false + } + return trimmed[0] == '{' || trimmed[0] == '[' +} diff --git a/internal/htmlsanitize/htmlsanitize_test.go b/internal/htmlsanitize/htmlsanitize_test.go new file mode 100644 --- /dev/null +++ b/internal/htmlsanitize/htmlsanitize_test.go @@ -0,0 +1,55 @@ +package htmlsanitize + +import ( + "bytes" + "encoding/json" + "html" + "testing" +) + +func TestJSONBodyEscapesStringValues(t *testing.T) { + t.Parallel() + + got, ok := JSONBody([]byte(`{"title":"","items":["safe & sound",{"description":"mode"}],"count":1}`)) + if !ok { + t.Fatal("JSONBody() ok = false, want true") + } + + var body map[string]any + if errUnmarshal := json.Unmarshal(got, &body); errUnmarshal != nil { + t.Fatalf("Unmarshal() error = %v; body=%s", errUnmarshal, string(got)) + } + if body["title"] != html.EscapeString("") { + t.Fatalf("title = %q, want escaped", body["title"]) + } + items, okItems := body["items"].([]any) + if !okItems || len(items) != 2 { + t.Fatalf("items = %#v, want two items", body["items"]) + } + if items[0] != html.EscapeString("safe & sound") { + t.Fatalf("items[0] = %q, want escaped", items[0]) + } + nested, okNested := items[1].(map[string]any) + if !okNested { + t.Fatalf("items[1] = %#v, want object", items[1]) + } + if nested["description"] != html.EscapeString("mode") { + t.Fatalf("description = %q, want escaped", nested["description"]) + } + if body["count"] != float64(1) { + t.Fatalf("count = %#v, want unchanged number", body["count"]) + } +} + +func TestJSONBodyIfLikelySkipsNonJSONHTML(t *testing.T) { + t.Parallel() + + body := []byte("plugin") + got, ok := JSONBodyIfLikely(body, "text/html; charset=utf-8") + if ok { + t.Fatal("JSONBodyIfLikely() ok = true, want false") + } + if !bytes.Equal(got, body) { + t.Fatalf("body = %q, want unchanged %q", string(got), string(body)) + } +} diff --git a/internal/pluginhost/management.go b/internal/pluginhost/management.go --- a/internal/pluginhost/management.go +++ b/internal/pluginhost/management.go @@ -8,6 +8,7 @@ "net/http" "strings" + "github.com/router-for-me/CLIProxyAPI/v7/internal/htmlsanitize" "github.com/router-for-me/CLIProxyAPI/v7/sdk/pluginapi" log "github.com/sirupsen/logrus" ) @@ -255,6 +256,7 @@ http.Error(w, "plugin management handler failed", http.StatusBadGateway) return true } + resp.Body = escapeManagementResponseBody(resp) for keyHeader, values := range resp.Headers { for _, value := range values { @@ -328,6 +330,14 @@ } }() return record.route.Handler.HandleManagement(ctx, req) +} + +func escapeManagementResponseBody(resp pluginapi.ManagementResponse) []byte { + body, okEscaped := htmlsanitize.JSONBodyIfLikely(resp.Body, resp.Headers.Get("Content-Type")) + if !okEscaped { + return resp.Body + } + return body } func (h *Host) callResourceHandler(ctx context.Context, record resourceRouteRecord, req pluginapi.ManagementRequest) (resp pluginapi.ManagementResponse, err error) { diff --git a/internal/pluginhost/management_test.go b/internal/pluginhost/management_test.go --- a/internal/pluginhost/management_test.go +++ b/internal/pluginhost/management_test.go @@ -2,6 +2,8 @@ import ( "context" + "encoding/json" + "html" "net/http" "net/http/httptest" "testing" @@ -60,6 +62,63 @@ rec = httptest.NewRecorder() if host.ServeManagementHTTP(rec, req) { t.Fatal("reserved route was served by plugin") + } +} + +func TestServeManagementHTMLEscapesJSONResponseStrings(t *testing.T) { + host := newHostWithRecords(capabilityRecord{ + id: "json", + plugin: pluginapi.Plugin{Capabilities: pluginapi.Capabilities{ + ManagementAPI: &managementPluginDouble{routes: []pluginapi.ManagementRoute{{ + Method: http.MethodGet, + Path: "/plugins/json/status", + Handler: managementHandlerFunc(func(context.Context, pluginapi.ManagementRequest) (pluginapi.ManagementResponse, error) { + return pluginapi.ManagementResponse{ + Headers: http.Header{"Content-Type": []string{"application/json; charset=utf-8"}}, + Body: []byte(`{ + "title": "", + "items": ["first", {"description": "safe & sound"}], + "count": 1 + }`), + }, nil + }), + }}}, + }}, + }) + host.RegisterManagementRoutes(context.Background(), nil) + + req := httptest.NewRequest(http.MethodGet, "/v0/management/plugins/json/status", nil) + rec := httptest.NewRecorder() + if !host.ServeManagementHTTP(rec, req) { + t.Fatal("ServeManagementHTTP() = false, want true") + } + if rec.Code != http.StatusOK { + t.Fatalf("status = %d, want 200; body=%s", rec.Code, rec.Body.String()) + } + + var body map[string]any + if errDecode := json.Unmarshal(rec.Body.Bytes(), &body); errDecode != nil { + t.Fatalf("Unmarshal() error = %v; body=%s", errDecode, rec.Body.String()) + } + if body["title"] != html.EscapeString("") { + t.Fatalf("title = %q, want escaped", body["title"]) + } + items, okItems := body["items"].([]any) + if !okItems || len(items) != 2 { + t.Fatalf("items = %#v, want two items", body["items"]) + } + if items[0] != html.EscapeString("first") { + t.Fatalf("items[0] = %q, want escaped", items[0]) + } + nested, okNested := items[1].(map[string]any) + if !okNested { + t.Fatalf("items[1] = %#v, want object", items[1]) + } + if nested["description"] != html.EscapeString("safe & sound") { + t.Fatalf("nested description = %q, want escaped", nested["description"]) + } + if body["count"] != float64(1) { + t.Fatalf("count = %#v, want unchanged number", body["count"]) } } diff --git a/internal/api/handlers/management/plugin_store.go b/internal/api/handlers/management/plugin_store.go --- a/internal/api/handlers/management/plugin_store.go +++ b/internal/api/handlers/management/plugin_store.go @@ -9,6 +9,7 @@ "github.com/gin-gonic/gin" "github.com/router-for-me/CLIProxyAPI/v7/internal/config" + "github.com/router-for-me/CLIProxyAPI/v7/internal/htmlsanitize" "github.com/router-for-me/CLIProxyAPI/v7/internal/pluginhost" "github.com/router-for-me/CLIProxyAPI/v7/internal/pluginstore" "github.com/router-for-me/CLIProxyAPI/v7/internal/util" @@ -81,19 +82,19 @@ status := statuses[plugin.ID] installedVersion := status.InstalledVersion entries = append(entries, pluginStoreListEntry{ - ID: plugin.ID, - Name: plugin.Name, - Description: plugin.Description, - Author: plugin.Author, - Version: plugin.Version, - Repository: plugin.Repository, - Logo: plugin.Logo, - Homepage: plugin.Homepage, - License: plugin.License, - Tags: append([]string{}, plugin.Tags...), + ID: htmlsanitize.String(plugin.ID), + Name: htmlsanitize.String(plugin.Name), + Description: htmlsanitize.String(plugin.Description), + Author: htmlsanitize.String(plugin.Author), + Version: htmlsanitize.String(plugin.Version), + Repository: htmlsanitize.String(plugin.Repository), + Logo: htmlsanitize.String(plugin.Logo), + Homepage: htmlsanitize.String(plugin.Homepage), + License: htmlsanitize.String(plugin.License), + Tags: htmlsanitize.Strings(plugin.Tags), Installed: status.Installed, - InstalledVersion: installedVersion, - Path: status.Path, + InstalledVersion: htmlsanitize.String(installedVersion), + Path: htmlsanitize.String(status.Path), Configured: status.Configured, Registered: status.Registered, Enabled: status.Enabled, @@ -104,7 +105,7 @@ c.JSON(http.StatusOK, pluginStoreListResponse{ PluginsEnabled: pluginsEnabled, - PluginsDir: pluginsDir, + PluginsDir: htmlsanitize.String(pluginsDir), Plugins: entries, }) } @@ -217,9 +218,9 @@ c.JSON(http.StatusOK, pluginInstallResponse{ Status: "installed", - ID: result.ID, - Version: result.Version, - Path: result.Path, + ID: htmlsanitize.String(result.ID), + Version: htmlsanitize.String(result.Version), + Path: htmlsanitize.String(result.Path), PluginsEnabled: pluginsEnabled, RestartRequired: restartRequired, }) diff --git a/internal/api/handlers/management/plugin_store_test.go b/internal/api/handlers/management/plugin_store_test.go --- a/internal/api/handlers/management/plugin_store_test.go +++ b/internal/api/handlers/management/plugin_store_test.go @@ -7,6 +7,7 @@ "crypto/sha256" "encoding/hex" "encoding/json" + "html" "io" "net/http" "net/http/httptest" @@ -76,6 +77,72 @@ } if entry.Path == "" { t.Fatal("path is empty") + } +} + +func TestListPluginStoreEscapesRegistryStrings(t *testing.T) { + t.Parallel() + gin.SetMode(gin.TestMode) + + h := &Handler{ + cfg: &config.Config{ + Plugins: config.PluginsConfig{ + Enabled: true, + Dir: t.TempDir(), + }, + }, + configFilePath: writeTestConfigFile(t), + pluginStoreRegistryURL: "https://registry.example/registry.json", + pluginStoreHTTPClient: fakePluginStoreHTTPClient{ + "https://registry.example/registry.json": []byte(`{ + "schema_version": 1, + "plugins": [{ + "id": "sample-provider", + "name": "", + "description": "", + "author": "\"attacker\"", + "version": "0.1.0", + "repository": "https://github.com/author-name/cliproxy-sample-provider-plugin", + "logo": "", + "homepage": "https://example.com/?q=", + "license": "MIT", + "tags": ["", "safe & sound"] + }] + }`), + }, + } + + rec := httptest.NewRecorder() + c, _ := gin.CreateTestContext(rec) + c.Request = httptest.NewRequest(http.MethodGet, "/v0/management/plugin-store", nil) + + h.ListPluginStore(c) + + if rec.Code != http.StatusOK { + t.Fatalf("status = %d, want %d; body=%s", rec.Code, http.StatusOK, rec.Body.String()) + } + var body pluginStoreListResponse + if errDecode := json.Unmarshal(rec.Body.Bytes(), &body); errDecode != nil { + t.Fatalf("Unmarshal() error = %v; body=%s", errDecode, rec.Body.String()) + } + if len(body.Plugins) != 1 { + t.Fatalf("plugins len = %d, want 1", len(body.Plugins)) + } + entry := body.Plugins[0] + if entry.Name != html.EscapeString("") || + entry.Description != html.EscapeString("") || + entry.Author != html.EscapeString(`"attacker"`) || + entry.Version != "0.1.0" || + entry.Repository != "https://github.com/author-name/cliproxy-sample-provider-plugin" || + entry.Logo != html.EscapeString("") || + entry.Homepage != html.EscapeString("https://example.com/?q=") || + entry.License != html.EscapeString("MIT") { + t.Fatalf("store entry = %#v, want escaped strings", entry) + } + if len(entry.Tags) != 2 || + entry.Tags[0] != html.EscapeString("") || + entry.Tags[1] != html.EscapeString("safe & sound") { + t.Fatalf("tags = %#v, want escaped strings", entry.Tags) } } diff --git a/internal/api/handlers/management/plugins.go b/internal/api/handlers/management/plugins.go --- a/internal/api/handlers/management/plugins.go +++ b/internal/api/handlers/management/plugins.go @@ -10,6 +10,7 @@ "github.com/gin-gonic/gin" "github.com/router-for-me/CLIProxyAPI/v7/internal/config" + "github.com/router-for-me/CLIProxyAPI/v7/internal/htmlsanitize" "github.com/router-for-me/CLIProxyAPI/v7/internal/pluginhost" "github.com/router-for-me/CLIProxyAPI/v7/sdk/pluginapi" "gopkg.in/yaml.v3" @@ -85,8 +86,8 @@ } for _, file := range files { entries[file.ID] = pluginListEntry{ - ID: file.ID, - Path: file.Path, + ID: htmlsanitize.String(file.ID), + Path: htmlsanitize.String(file.Path), Enabled: true, ConfigFields: []pluginConfigFieldInfo{}, Menus: []pluginMenuInfo{}, @@ -94,7 +95,7 @@ } for id, item := range configs { entry := entries[id] - entry.ID = id + entry.ID = htmlsanitize.String(id) entry.Configured = true entry.Enabled = pluginInstanceEnabled(item) if entry.ConfigFields == nil { @@ -108,10 +109,10 @@ if host != nil { for _, info := range host.RegisteredPlugins() { entry := entries[info.ID] - entry.ID = info.ID + entry.ID = htmlsanitize.String(info.ID) entry.Registered = true entry.SupportsOAuth = info.SupportsOAuth - entry.Logo = info.Metadata.Logo + entry.Logo = htmlsanitize.String(info.Metadata.Logo) entry.ConfigFields = pluginConfigFields(info.Metadata.ConfigFields) entry.Menus = pluginMenus(info.Menus) entry.Metadata = pluginMetadata(info.Metadata) @@ -143,7 +144,7 @@ c.JSON(http.StatusOK, pluginListResponse{ PluginsEnabled: pluginsEnabled, - PluginsDir: pluginsDir, + PluginsDir: htmlsanitize.String(pluginsDir), Plugins: out, }) } @@ -265,12 +266,11 @@ func pluginConfigFields(fields []pluginapi.ConfigField) []pluginConfigFieldInfo { out := make([]pluginConfigFieldInfo, 0, len(fields)) for _, field := range fields { - enumValues := append([]string{}, field.EnumValues...) out = append(out, pluginConfigFieldInfo{ - Name: field.Name, - Type: string(field.Type), - EnumValues: enumValues, - Description: field.Description, + Name: htmlsanitize.String(field.Name), + Type: htmlsanitize.String(string(field.Type)), + EnumValues: htmlsanitize.Strings(field.EnumValues), + Description: htmlsanitize.String(field.Description), }) } return out @@ -280,9 +280,9 @@ out := make([]pluginMenuInfo, 0, len(menus)) for _, menu := range menus { out = append(out, pluginMenuInfo{ - Path: menu.Path, - Menu: menu.Menu, - Description: menu.Description, + Path: htmlsanitize.String(menu.Path), + Menu: htmlsanitize.String(menu.Menu), + Description: htmlsanitize.String(menu.Description), }) } return out @@ -290,11 +290,11 @@ func pluginMetadata(meta pluginapi.Metadata) *pluginMetadataInfo { return &pluginMetadataInfo{ - Name: meta.Name, - Version: meta.Version, - Author: meta.Author, - GitHubRepository: meta.GitHubRepository, - Logo: meta.Logo, + Name: htmlsanitize.String(meta.Name), + Version: htmlsanitize.String(meta.Version), + Author: htmlsanitize.String(meta.Author), + GitHubRepository: htmlsanitize.String(meta.GitHubRepository), + Logo: htmlsanitize.String(meta.Logo), ConfigFields: pluginConfigFields(meta.ConfigFields), } } diff --git a/internal/api/handlers/management/plugins_test.go b/internal/api/handlers/management/plugins_test.go --- a/internal/api/handlers/management/plugins_test.go +++ b/internal/api/handlers/management/plugins_test.go @@ -3,6 +3,7 @@ import ( "bytes" "encoding/json" + "html" "net/http" "net/http/httptest" "os" @@ -13,6 +14,8 @@ "github.com/gin-gonic/gin" "github.com/router-for-me/CLIProxyAPI/v7/internal/config" + "github.com/router-for-me/CLIProxyAPI/v7/internal/pluginhost" + "github.com/router-for-me/CLIProxyAPI/v7/sdk/pluginapi" "gopkg.in/yaml.v3" ) @@ -208,6 +211,58 @@ raw := marshalPluginRaw(t, item) if !strings.Contains(raw, "mode: fast") || !strings.Contains(raw, "count: 3") || strings.Contains(raw, "remove:") { t.Fatalf("raw config =\n%s", raw) + } +} + +func TestPluginDisplayFieldsEscapeHTML(t *testing.T) { + t.Parallel() + + fields := pluginConfigFields([]pluginapi.ConfigField{{ + Name: ``, + Type: pluginapi.ConfigFieldTypeEnum, + EnumValues: []string{``, `safe & sound`}, + Description: `"quoted" 'single' mode`, + }}) + if len(fields) != 1 { + t.Fatalf("fields len = %d, want 1", len(fields)) + } + if fields[0].Name != html.EscapeString(``) { + t.Fatalf("field name = %q, want escaped", fields[0].Name) + } + if fields[0].EnumValues[0] != html.EscapeString(``) || fields[0].EnumValues[1] != html.EscapeString(`safe & sound`) { + t.Fatalf("enum values = %#v, want escaped values", fields[0].EnumValues) + } + if fields[0].Description != html.EscapeString(`"quoted" 'single' mode`) { + t.Fatalf("description = %q, want escaped", fields[0].Description) + } + + menus := pluginMenus([]pluginhost.RegisteredPluginMenu{{ + Path: `/v0/resource/plugins/sample/`, + Menu: `Status`, + Description: `Shows .`, + }}) + if len(menus) != 1 { + t.Fatalf("menus len = %d, want 1", len(menus)) + } + if menus[0].Path != html.EscapeString(`/v0/resource/plugins/sample/`) || + menus[0].Menu != html.EscapeString(`Status`) || + menus[0].Description != html.EscapeString(`Shows .`) { + t.Fatalf("menu = %#v, want escaped strings", menus[0]) + } + + meta := pluginMetadata(pluginapi.Metadata{ + Name: ``, + Version: `1.0.0&evil=true`, + Author: `"attacker"`, + GitHubRepository: `https://example.com/repo?x=`) || + meta.Version != html.EscapeString(`1.0.0&evil=true`) || + meta.Author != html.EscapeString(`"attacker"`) || + meta.GitHubRepository != html.EscapeString(`https://example.com/repo?x=