all: Sanitize HTML in error responses to prevent reflected XSS

Author: nicholaspcr

pkg/rpcserver/rpcserver.go

@@ -18,6 +18,7 @@ package rpcserver
 import (
 	"context"
 	"fmt"
+	"html"
 	"math"
 	"net/http"
 	"net/textproto"
@@ -50,6 +51,7 @@ import (
 	_ "google.golang.org/grpc/encoding/gzip" // Register gzip compression.
 	"google.golang.org/grpc/keepalive"
 	"google.golang.org/grpc/metadata"
+	grpcstatus "google.golang.org/grpc/status"
 )
 
 func init() {
@@ -132,6 +134,31 @@ func WithRateLimiter(limiter ratelimit.Interface) Option {
 // ErrRPCRecovered is returned when a panic is caught from an RPC.
 var ErrRPCRecovered = errors.DefineInternal("rpc_recovered", "Internal Server Error")
 
+// sanitizingHTTPErrorHandler wraps runtime.DefaultHTTPErrorHandler to HTML-escape
+// gRPC status messages before they are serialized to JSON. This prevents reflected
+// XSS from user-supplied input that flows into error messages (e.g., field mask paths,
+// strconv.ParseBool parse errors, route-not-found paths).
+//
+// Note: ErrorDetailsToProto separately sanitizes error attributes in the details
+// payload. This handler covers the top-level "message" field which contains the
+// formatted error string with raw attribute values.
+func sanitizingHTTPErrorHandler(
+	ctx context.Context,
+	mux *runtime.ServeMux,
+	marshaler runtime.Marshaler,
+	w http.ResponseWriter,
+	r *http.Request,
+	err error,
+) {
+	if st, ok := grpcstatus.FromError(err); ok {
+		// Rebuild the status with escaped message while preserving details.
+		pb := st.Proto()
+		pb.Message = html.EscapeString(pb.Message)
+		err = grpcstatus.ErrorProto(pb)
+	}
+	runtime.DefaultHTTPErrorHandler(ctx, mux, marshaler, w, r, err)
+}
+
 // New returns a new RPC server with a set of middlewares.
 // The given context is used in some of the middlewares, the given server options are passed to gRPC
 //
@@ -234,7 +261,7 @@ func New(ctx context.Context, opts ...Option) *Server {
 	server.ServeMux = runtime.NewServeMux(
 		runtime.WithMarshalerOption("*", jsonpb.TTN()),
 		runtime.WithMarshalerOption("text/event-stream", jsonpb.TTNEventStream()),
-		runtime.WithErrorHandler(runtime.DefaultHTTPErrorHandler),
+		runtime.WithErrorHandler(sanitizingHTTPErrorHandler),
 		runtime.WithMetadata(func(ctx context.Context, req *http.Request) metadata.MD {
 			md := rpcmetadata.MD{
 				Host: req.Host,

pkg/rpcserver/rpcserver_test.go

@@ -16,6 +16,10 @@ package rpcserver_test
 
 import (
 	"context"
+	"encoding/json"
+	"io"
+	"net/http"
+	"net/http/httptest"
 	"runtime"
 	"testing"
 	"time"
@@ -173,3 +177,38 @@ func (s *mockServer) DownlinkQueuePush(ctx context.Context, req *ttnpb.DownlinkQ
 	s.pushCtx, s.pushReq = ctx, req
 	return ttnpb.Empty, nil
 }
+
+func TestSanitizingHTTPErrorHandler(t *testing.T) {
+	t.Parallel()
+	a := assertions.New(t)
+	ctx := test.Context()
+
+	logHandler := memory.New()
+	logger := log.NewLogger(logHandler)
+	ctx = log.NewContext(ctx, logger)
+
+	server := rpcserver.New(ctx)
+
+	// Request a non-existent gRPC-gateway route with XSS payload in the path.
+	xssPath := `/api/v3/<script>alert("xss")</script>`
+	r := httptest.NewRequest(http.MethodGet, xssPath, nil)
+	r.Header.Set("Content-Type", "application/json")
+	rec := httptest.NewRecorder()
+
+	server.ServeHTTP(rec, r)
+
+	res := rec.Result()
+	body, _ := io.ReadAll(res.Body)
+
+	// Verify the response is valid JSON with no unescaped script tags.
+	var resp map[string]any
+	a.So(json.Unmarshal(body, &resp), should.BeNil)
+
+	// Check the message field if present.
+	if msg, ok := resp["message"].(string); ok {
+		a.So(msg, should.NotContainSubstring, "<script>")
+	}
+
+	// Also verify no unescaped tags in the raw body.
+	a.So(string(body), should.NotContainSubstring, "<script>")
+}

pkg/ttnpb/errors.go

@@ -16,6 +16,7 @@ package ttnpb
 
 import (
 	"fmt"
+	"html"
 
 	"go.thethings.network/lorawan-stack/v3/pkg/errors"
 	"go.thethings.network/lorawan-stack/v3/pkg/goproto"
@@ -69,6 +70,44 @@ func (e errorDetails) Details() []proto.Message {
 	return msgs
 }
 
+// sanitizeAttributeValue HTML-escapes strings within attribute values
+// to prevent reflected XSS when error responses are rendered as HTML.
+// Returns new values without mutating the originals.
+func sanitizeAttributeValue(v any) any {
+	switch val := v.(type) {
+	case string:
+		return html.EscapeString(val)
+	case []string:
+		escaped := make([]string, len(val))
+		for i, s := range val {
+			escaped[i] = html.EscapeString(s)
+		}
+		return escaped
+	case []any:
+		escaped := make([]any, len(val))
+		for i, item := range val {
+			escaped[i] = sanitizeAttributeValue(item)
+		}
+		return escaped
+	case map[string]any:
+		return sanitizeAttributes(val)
+	default:
+		return v
+	}
+}
+
+// sanitizeAttributes returns a new attribute map with all string values HTML-escaped.
+func sanitizeAttributes(attrs map[string]any) map[string]any {
+	if len(attrs) == 0 {
+		return attrs
+	}
+	sanitized := make(map[string]any, len(attrs))
+	for k, v := range attrs {
+		sanitized[html.EscapeString(k)] = sanitizeAttributeValue(v)
+	}
+	return sanitized
+}
+
 func ErrorDetailsToProto(e errors.ErrorDetails) *ErrorDetails {
 	pb := &ErrorDetails{
 		Namespace:     e.Namespace(),
@@ -77,7 +116,7 @@ func ErrorDetailsToProto(e errors.ErrorDetails) *ErrorDetails {
 		CorrelationId: e.CorrelationID(),
 		Code:          e.Code(),
 	}
-	if attributes := e.PublicAttributes(); len(attributes) > 0 {
+	if attributes := sanitizeAttributes(e.PublicAttributes()); len(attributes) > 0 {
 		attributesStruct, err := goproto.Struct(attributes)
 		if err != nil {
 			panic(fmt.Sprintf("Failed to encode error attributes: %s", err)) // Likely a bug in ttn (invalid attribute type).

pkg/ttnpb/errors_test.go

@@ -19,9 +19,53 @@ import (
 
 	"github.com/smarty/assertions"
 	"go.thethings.network/lorawan-stack/v3/pkg/errors"
+	"go.thethings.network/lorawan-stack/v3/pkg/goproto"
+	"go.thethings.network/lorawan-stack/v3/pkg/ttnpb"
 	"go.thethings.network/lorawan-stack/v3/pkg/util/test/assertions/should"
 )
 
+func TestErrorDetailsToProtoXSSSanitization(t *testing.T) {
+	t.Parallel()
+	a := assertions.New(t)
+
+	errDef := errors.Define(
+		"test_xss_sanitization",
+		"XSS Test Error",
+		"path", "paths", "count",
+	)
+
+	xssPayload := `<script>alert("xss")</script>`
+
+	errWithXSS := errDef.WithAttributes(
+		"path", xssPayload,
+		"paths", []string{xssPayload, "safe"},
+		"count", 42,
+	)
+
+	pb := ttnpb.ErrorDetailsToProto(errWithXSS)
+	a.So(pb, should.NotBeNil)
+
+	// Recover attributes from the proto to verify sanitization.
+	attrs, err := goproto.Map(pb.GetAttributes())
+	a.So(err, should.BeNil)
+
+	// String attribute must be escaped.
+	a.So(attrs["path"], should.NotContainSubstring, "<script>")
+	a.So(attrs["path"], should.ContainSubstring, "&lt;script&gt;")
+
+	// []string attribute values must be escaped.
+	if paths, ok := attrs["paths"].([]any); ok && len(paths) >= 2 {
+		a.So(paths[0], should.NotContainSubstring, "<script>")
+		a.So(paths[0], should.ContainSubstring, "&lt;script&gt;")
+		a.So(paths[1], should.Equal, "safe")
+	} else {
+		t.Fatal("expected paths attribute to be a list with 2 elements")
+	}
+
+	// Numeric attribute must pass through unchanged.
+	a.So(attrs["count"], should.Equal, float64(42))
+}
+
 func TestGRPCConversion(t *testing.T) {
 	a := assertions.New(t)
 

pkg/webhandlers/error_test.go

@@ -15,6 +15,7 @@
 package webhandlers_test
 
 import (
+	"encoding/json"
 	"io"
 	"net/http"
 	"net/http/httptest"
@@ -27,6 +28,38 @@ import (
 	. "go.thethings.network/lorawan-stack/v3/pkg/webhandlers"
 )
 
+func TestNotFoundXSSSanitization(t *testing.T) {
+	t.Parallel()
+	a := assertions.New(t)
+
+	xssPath := `/api/v3/<script>alert("xss")</script>`
+
+	r := httptest.NewRequest(http.MethodGet, xssPath, nil)
+	rec := httptest.NewRecorder()
+
+	NotFound(rec, r)
+
+	res := rec.Result()
+	body, _ := io.ReadAll(res.Body)
+
+	a.So(res.StatusCode, should.Equal, http.StatusNotFound)
+
+	// Decode the JSON to inspect the attribute value after JSON decoding.
+	var resp map[string]any
+	a.So(json.Unmarshal(body, &resp), should.BeNil)
+
+	// Extract the route attribute from the error details.
+	details, _ := resp["details"].([]any) //nolint:revive
+	a.So(len(details), should.BeGreaterThan, 0)
+	detail, _ := details[0].(map[string]any)          //nolint:revive
+	attrs, _ := detail["attributes"].(map[string]any) //nolint:revive
+	route, _ := attrs["route"].(string)               //nolint:revive
+
+	// The attribute value must be HTML-escaped, not the raw XSS payload.
+	a.So(route, should.NotContainSubstring, "<script>")
+	a.So(route, should.ContainSubstring, "&lt;script&gt;")
+}
+
 func TestErrorHandler(t *testing.T) {
 	ctx, getError := NewContextWithErrorValue(test.Context())