Fix XSS vulnerabilities in snippet annotations and plugin template
Change-Id: I7cd476e4cddc785eff465d6f5595bdbbe8aa9f45
diff --git a/cmd/koralmapper/main.go b/cmd/koralmapper/main.go
index 75a1d9c..c1fbabe 100644
--- a/cmd/koralmapper/main.go
+++ b/cmd/koralmapper/main.go
@@ -14,7 +14,6 @@
"strconv"
"strings"
"syscall"
- texttemplate "text/template"
"time"
"github.com/KorAP/Koral-Mapper/config"
@@ -308,7 +307,7 @@
func setupRoutes(app *fiber.App, m *mapper.Mapper, yamlConfig *config.MappingConfig) {
configTmpl := template.Must(template.ParseFS(staticFS, "static/config.html"))
- pluginTmpl := texttemplate.Must(texttemplate.ParseFS(staticFS, "static/plugin.html"))
+ pluginTmpl := template.Must(template.ParseFS(staticFS, "static/plugin.html"))
// Health check endpoint
app.Get("/health", func(c *fiber.Ctx) error {
@@ -716,7 +715,7 @@
return nil
}
-func handleKalamarPlugin(yamlConfig *config.MappingConfig, configTmpl *template.Template, pluginTmpl *texttemplate.Template) fiber.Handler {
+func handleKalamarPlugin(yamlConfig *config.MappingConfig, configTmpl *template.Template, pluginTmpl *template.Template) fiber.Handler {
return func(c *fiber.Ctx) error {
mapID := c.Params("map")
diff --git a/cmd/koralmapper/main_test.go b/cmd/koralmapper/main_test.go
index 74e130d..4543c8e 100644
--- a/cmd/koralmapper/main_test.go
+++ b/cmd/koralmapper/main_test.go
@@ -21,6 +21,14 @@
"github.com/stretchr/testify/require"
)
+// jsEscapeURL converts a URL to its html/template JS string escaped form.
+// In JS string context, html/template escapes / as \/ and & as \u0026.
+func jsEscapeURL(u string) string {
+ u = strings.ReplaceAll(u, "/", `\/`)
+ u = strings.ReplaceAll(u, "&", `\u0026`)
+ return u
+}
+
func loadConfigFromYAML(t *testing.T, configYAML string, mappingYAMLs ...string) *tmconfig.MappingConfig {
t.Helper()
@@ -1240,8 +1248,8 @@
htmlContent := string(body)
- // Check that the HTML contains the expected service URL in the JavaScript
- expectedJSURL := tt.expectedServiceURL + "/test-mapper/query"
+ // html/template applies JS string escaping (/ -> \/, & -> \u0026)
+ expectedJSURL := jsEscapeURL(tt.expectedServiceURL + "/test-mapper/query")
assert.Contains(t, htmlContent, "'service' : '"+expectedJSURL)
// Ensure it's still a valid HTML page
@@ -1299,7 +1307,7 @@
require.NoError(t, err)
htmlContent := string(body)
- expectedJSURL := "https://custom.example.com/api/koralmapper/config-mapper/query"
+ expectedJSURL := jsEscapeURL("https://custom.example.com/api/koralmapper/config-mapper/query")
assert.Contains(t, htmlContent, "'service' : '"+expectedJSURL)
}
@@ -1364,7 +1372,7 @@
require.NoError(t, err)
htmlContent := string(body)
- expectedJSURL := "https://korap.ids-mannheim.de/plugin/koralmapper/main-config-mapper/query"
+ expectedJSURL := jsEscapeURL("https://korap.ids-mannheim.de/plugin/koralmapper/main-config-mapper/query")
assert.Contains(t, htmlContent, "'service' : '"+expectedJSURL)
}
@@ -1545,9 +1553,9 @@
} else {
htmlContent := string(body)
- // Check that both query and response URLs are present with correct parameters
- assert.Contains(t, htmlContent, "'service' : '"+tt.expectedQueryURL+"'")
- assert.Contains(t, htmlContent, "'service' : '"+tt.expectedRespURL+"'")
+ // html/template applies JS string escaping in script contexts
+ assert.Contains(t, htmlContent, "'service' : '"+jsEscapeURL(tt.expectedQueryURL)+"'")
+ assert.Contains(t, htmlContent, "'service' : '"+jsEscapeURL(tt.expectedRespURL)+"'")
// Ensure it's still a valid HTML page
assert.Contains(t, htmlContent, "Koral-Mapper")
@@ -2112,6 +2120,83 @@
assert.Contains(t, htmlContent, `placeholder="topic"`)
}
+// TestPluginPageEscapesMapID verifies that the plugin page (GET /:map)
+// properly HTML-escapes the map ID to prevent XSS via URL path injection.
+func TestPluginPageEscapesMapID(t *testing.T) {
+ mappingList := tmconfig.MappingList{
+ ID: "test-mapper",
+ Mappings: []tmconfig.MappingRule{"[A] <> [B]"},
+ }
+
+ m, err := mapper.NewMapper([]tmconfig.MappingList{mappingList})
+ require.NoError(t, err)
+
+ mockConfig := &tmconfig.MappingConfig{
+ ServiceURL: "https://example.com/plugin/koralmapper",
+ Lists: []tmconfig.MappingList{mappingList},
+ }
+ tmconfig.ApplyDefaults(mockConfig)
+
+ app := fiber.New()
+ setupRoutes(app, m, mockConfig)
+
+ // Use a map ID that contains HTML/JS injection payload
+ maliciousMapID := `"><script>alert(1)</script>`
+ req := httptest.NewRequest(http.MethodGet, "/"+maliciousMapID, nil)
+ resp, err := app.Test(req)
+ require.NoError(t, err)
+ defer resp.Body.Close()
+
+ // The request may fail validation (contains invalid chars), which is also acceptable.
+ // If it renders, the output must not contain unescaped script tags.
+ if resp.StatusCode == http.StatusOK {
+ body, err := io.ReadAll(resp.Body)
+ require.NoError(t, err)
+ htmlContent := string(body)
+
+ // The raw script tag must NOT appear in the output
+ assert.NotContains(t, htmlContent, "<script>alert(1)</script>")
+ // If rendered, it should be escaped
+ assert.Contains(t, htmlContent, "<script>")
+ }
+}
+
+// TestPluginPageEscapesServiceURL verifies that the plugin page properly
+// escapes ServiceURL values to prevent template injection.
+func TestPluginPageEscapesServiceURL(t *testing.T) {
+ mappingList := tmconfig.MappingList{
+ ID: "test-mapper",
+ Mappings: []tmconfig.MappingRule{"[A] <> [B]"},
+ }
+
+ m, err := mapper.NewMapper([]tmconfig.MappingList{mappingList})
+ require.NoError(t, err)
+
+ // Inject a ServiceURL with HTML-special characters
+ mockConfig := &tmconfig.MappingConfig{
+ ServiceURL: `https://example.com/plugin" onload="alert(1)`,
+ Lists: []tmconfig.MappingList{mappingList},
+ }
+ tmconfig.ApplyDefaults(mockConfig)
+
+ app := fiber.New()
+ setupRoutes(app, m, mockConfig)
+
+ req := httptest.NewRequest(http.MethodGet, "/test-mapper", nil)
+ resp, err := app.Test(req)
+ require.NoError(t, err)
+ defer resp.Body.Close()
+
+ assert.Equal(t, http.StatusOK, resp.StatusCode)
+
+ body, err := io.ReadAll(resp.Body)
+ require.NoError(t, err)
+ htmlContent := string(body)
+
+ // The unescaped injection must NOT appear in the output
+ assert.NotContains(t, htmlContent, `onload="alert(1)"`)
+}
+
func TestConfigPageBackwardCompatibility(t *testing.T) {
lists := []tmconfig.MappingList{
{
@@ -2148,7 +2233,7 @@
assert.Contains(t, htmlContent, "Koral-Mapper")
assert.Contains(t, htmlContent, "Map ID: test-mapper")
assert.Contains(t, htmlContent, "KorAPlugin.sendMsg")
- assert.Contains(t, htmlContent, "test-mapper/query")
+ assert.Contains(t, htmlContent, `test-mapper\/query`)
}
func TestBuildConfigPageData(t *testing.T) {
diff --git a/cmd/koralmapper/static/plugin.html b/cmd/koralmapper/static/plugin.html
index b4cc149..019d0bc 100644
--- a/cmd/koralmapper/static/plugin.html
+++ b/cmd/koralmapper/static/plugin.html
@@ -41,7 +41,6 @@
</div>
<script>
- <!-- activates/deactivates Mapper. -->
let qdata = {
'action' : 'pipe',
'service' : '{{.QueryURL}}'
diff --git a/mapper/response.go b/mapper/response.go
index 3ac6b02..61ff94b 100644
--- a/mapper/response.go
+++ b/mapper/response.go
@@ -2,6 +2,7 @@
import (
"fmt"
+ "html"
"maps"
"strings"
@@ -253,7 +254,7 @@
annotated := escapeXMLText(trimmed)
for i := len(annotationStrings) - 1; i >= 0; i-- {
- annotated = fmt.Sprintf(`<span title="%s" class="notinindex">%s</span>`, annotationStrings[i], annotated)
+ annotated = fmt.Sprintf(`<span title="%s" class="notinindex">%s</span>`, html.EscapeString(annotationStrings[i]), annotated)
}
result.WriteString(annotated)
result.WriteString(trailingWS)
diff --git a/mapper/response_test.go b/mapper/response_test.go
index 9a0c668..8452730 100644
--- a/mapper/response_test.go
+++ b/mapper/response_test.go
@@ -931,6 +931,84 @@
assert.Contains(t, snippet, `<span title="opennlp/p:NOUN" class="notinindex">Mann</span>`)
}
+// TestResponseAnnotationHTMLEscaping verifies that annotation strings containing
+// HTML-special characters are properly escaped in the title attribute to prevent XSS.
+func TestResponseAnnotationHTMLEscaping(t *testing.T) {
+ responseSnippet := `{
+ "snippet": "<span title=\"marmot/p:DET\">Der</span>"
+ }`
+
+ // Mapping rule where the replacement foundry contains a quote character
+ // that could break out of the HTML title attribute if unescaped.
+ mappingList := config.MappingList{
+ ID: "test-xss-mapper",
+ FoundryA: "marmot",
+ LayerA: "p",
+ FoundryB: `foo" onmouseover="alert(1)" x="`,
+ LayerB: "p",
+ Mappings: []config.MappingRule{
+ "[DET] <> [DT]",
+ },
+ }
+
+ m, err := NewMapper([]config.MappingList{mappingList})
+ require.NoError(t, err)
+
+ var inputData any
+ err = json.Unmarshal([]byte(responseSnippet), &inputData)
+ require.NoError(t, err)
+
+ result, err := m.ApplyResponseMappings("test-xss-mapper", MappingOptions{Direction: AtoB}, inputData)
+ require.NoError(t, err)
+
+ resultMap := result.(map[string]any)
+ snippet := resultMap["snippet"].(string)
+
+ // The quote character MUST be escaped (as " or ") in the title attribute
+ // so it cannot break out. The raw unescaped sequence must not appear.
+ assert.NotContains(t, snippet, `title="foo" onmouseover="alert(1)"`)
+ // The escaped version should be present (" is the html.EscapeString encoding for ")
+ assert.Contains(t, snippet, `"`)
+ assert.Contains(t, snippet, `class="notinindex"`)
+}
+
+// TestResponseAnnotationHTMLEscapingAngleBrackets verifies that angle brackets
+// in annotation strings are escaped to prevent HTML injection.
+func TestResponseAnnotationHTMLEscapingAngleBrackets(t *testing.T) {
+ responseSnippet := `{
+ "snippet": "<span title=\"marmot/p:DET\">Der</span>"
+ }`
+
+ mappingList := config.MappingList{
+ ID: "test-angle-mapper",
+ FoundryA: "marmot",
+ LayerA: "p",
+ FoundryB: "<script>",
+ LayerB: "p",
+ Mappings: []config.MappingRule{
+ "[DET] <> [DT]",
+ },
+ }
+
+ m, err := NewMapper([]config.MappingList{mappingList})
+ require.NoError(t, err)
+
+ var inputData any
+ err = json.Unmarshal([]byte(responseSnippet), &inputData)
+ require.NoError(t, err)
+
+ result, err := m.ApplyResponseMappings("test-angle-mapper", MappingOptions{Direction: AtoB}, inputData)
+ require.NoError(t, err)
+
+ resultMap := result.(map[string]any)
+ snippet := resultMap["snippet"].(string)
+
+ // Angle brackets must be escaped in the title attribute
+ assert.NotContains(t, snippet, `<script>`)
+ assert.Contains(t, snippet, `<script>`)
+ assert.Contains(t, snippet, `class="notinindex"`)
+}
+
// TestResponseMappingWithLayerOverride tests layer precedence rules
func TestResponseMappingWithLayerOverride(t *testing.T) {
// Test 1: Explicit layer in mapping rule should take precedence over MappingOptions