From a80e3cf855878673a798401c55395f3076dd1fb5 Mon Sep 17 00:00:00 2001 From: EROMOSELE AKHIGBE Date: Fri, 15 Mar 2024 13:34:01 +0100 Subject: [PATCH 1/4] modified FN in templates.go to use auto-escaping HTML --- CHANGELOG.md | 2 ++ zpages/templates.go | 16 ++++++++++++++-- 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 479077cfbde..2df22ab9069 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,8 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm ### Added - Add the new `go.opentelemetry.io/contrib/instrgen` package to provide auto-generated source code instrumentation. (#3068, #3108) +- Add a new package `go.opentelemetry.io/contrib/zpages/internal` to enable the use of internal.Templates within the parseTemplate function. +- Refactored the spanRowFormatter function to utilize HTML templates for auto-escaping HTML, ensuring protection against Cross-site Scripting (XSS) vulnerabilities. (#4451) ### Removed diff --git a/zpages/templates.go b/zpages/templates.go index ca23135adfd..22340a95a76 100644 --- a/zpages/templates.go +++ b/zpages/templates.go @@ -19,6 +19,7 @@ package zpages // import "go.opentelemetry.io/contrib/zpages" import ( + "bytes" "fmt" "html/template" "io" @@ -69,10 +70,21 @@ func spanRowFormatter(r spanRow) template.HTML { if r.SpanContext.IsSampled() { col = "blue" } + + var tpl string + if r.ParentSpanContext.IsValid() { - return template.HTML(fmt.Sprintf(`trace_id: %s span_id: %s parent_span_id: %s`, col, r.SpanContext.TraceID(), r.SpanContext.SpanID(), r.ParentSpanContext.SpanID())) + tpl = fmt.Sprintf(`trace_id: %s span_id: %s parent_span_id: %s`, col, r.SpanContext.TraceID(), r.SpanContext.SpanID(), r.ParentSpanContext.SpanID()) + } else { + tpl = fmt.Sprintf(`trace_id: %s span_id: %s `, col, r.SpanContext.TraceID(), r.SpanContext.SpanID()) + } + + t := template.Must(template.New("spanRow").Parse(tpl)) + var buf bytes.Buffer + if err := t.Execute(&buf, nil); err != nil { + return template.HTML(fmt.Sprintf("Error executing template: %v", err)) } - return template.HTML(fmt.Sprintf(`trace_id: %s span_id: %s`, col, r.SpanContext.TraceID(), r.SpanContext.SpanID())) + return template.HTML(buf.String()) } func even(x int) bool { From 1febcd61f4264310d0f084f0dfca0d902e75b985 Mon Sep 17 00:00:00 2001 From: EROMOSELE AKHIGBE Date: Fri, 15 Mar 2024 13:36:59 +0100 Subject: [PATCH 2/4] removed comment --- zpages/templates.go | 1 - 1 file changed, 1 deletion(-) diff --git a/zpages/templates.go b/zpages/templates.go index 22340a95a76..8ffabfdbb8c 100644 --- a/zpages/templates.go +++ b/zpages/templates.go @@ -61,7 +61,6 @@ func parseTemplate(name string) *template.Template { return template.Must(template.New(name).Funcs(templateFunctions).Parse(string(text))) } -//nolint:gosec // G203: The used method does not auto-escape HTML. Tracked under https://github.com/open-telemetry/opentelemetry-go-contrib/issues/4451. func spanRowFormatter(r spanRow) template.HTML { if !r.SpanContext.IsValid() { return "" From 51899df3c463d8b16c07ffc19036533ae3e6ba32 Mon Sep 17 00:00:00 2001 From: EROMOSELE AKHIGBE Date: Sat, 16 Mar 2024 00:30:49 +0100 Subject: [PATCH 3/4] updated template.go --- zpages/templates.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/zpages/templates.go b/zpages/templates.go index 8ffabfdbb8c..cc21d931c68 100644 --- a/zpages/templates.go +++ b/zpages/templates.go @@ -73,17 +73,17 @@ func spanRowFormatter(r spanRow) template.HTML { var tpl string if r.ParentSpanContext.IsValid() { - tpl = fmt.Sprintf(`trace_id: %s span_id: %s parent_span_id: %s`, col, r.SpanContext.TraceID(), r.SpanContext.SpanID(), r.ParentSpanContext.SpanID()) + tpl = fmt.Sprintf(`trace_id: %s span_id: %s parent_span_id: %s`, col, template.HTMLEscapeString(r.SpanContext.TraceID().String()), template.HTMLEscapeString(r.SpanContext.SpanID().String()), template.HTMLEscapeString(r.ParentSpanContext.SpanID().String())) } else { - tpl = fmt.Sprintf(`trace_id: %s span_id: %s `, col, r.SpanContext.TraceID(), r.SpanContext.SpanID()) + tpl = fmt.Sprintf(`trace_id: %s span_id: %s `, col, template.HTMLEscapeString(r.SpanContext.TraceID().String()), template.HTMLEscapeString(r.SpanContext.SpanID().String())) } t := template.Must(template.New("spanRow").Parse(tpl)) var buf bytes.Buffer if err := t.Execute(&buf, nil); err != nil { - return template.HTML(fmt.Sprintf("Error executing template: %v", err)) + return template.HTML(template.HTMLEscapeString(fmt.Sprintf("Error executing template: %s", err.Error()))) } - return template.HTML(buf.String()) + return template.HTML(template.HTMLEscapeString(buf.String())) } func even(x int) bool { From 2f960f33866f7e2c9228421aed3ce085f8929a2d Mon Sep 17 00:00:00 2001 From: EROMOSELE AKHIGBE Date: Mon, 18 Mar 2024 23:15:03 +0100 Subject: [PATCH 4/4] updated changelog --- CHANGELOG.md | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2df22ab9069..794b74ef110 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,8 +11,10 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm ### Added - Add the new `go.opentelemetry.io/contrib/instrgen` package to provide auto-generated source code instrumentation. (#3068, #3108) -- Add a new package `go.opentelemetry.io/contrib/zpages/internal` to enable the use of internal.Templates within the parseTemplate function. -- Refactored the spanRowFormatter function to utilize HTML templates for auto-escaping HTML, ensuring protection against Cross-site Scripting (XSS) vulnerabilities. (#4451) + +### Fixed + +- Improved the spanRowFormatter function by using HTML templates that automatically handle HTML escaping. This change strengthens security against Cross-Site Scripting (XSS) vulnerabilities. (Issue #4451) ### Removed