Skip to content

Commit 9a5cf4b

Browse files
authored
Merge pull request #7849 from TheThingsNetwork/backport/potential-cross-site-scripting
Sanitize HTML in error responses to prevent reflected XSS
2 parents aa705c5 + 1598ef1 commit 9a5cf4b

6 files changed

Lines changed: 188 additions & 2 deletions

File tree

CHANGELOG.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,10 @@ For details about compatibility between different releases, see the **Commitment
2727

2828
- The timestamp of the udp packet is now always correct when the 'Schedule downlink late' is enabled for the gateway and downlink scheduling hits the duty cycle limit.
2929

30+
### Security
31+
32+
- HTML-escape gRPC error messages and error detail attributes to prevent reflected XSS in API error responses.
33+
3034
## [3.35.2] - 2026-01-30
3135

3236
### Changed

pkg/rpcserver/rpcserver.go

Lines changed: 28 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ package rpcserver
1818
import (
1919
"context"
2020
"fmt"
21+
"html"
2122
"math"
2223
"net/http"
2324
"net/textproto"
@@ -50,6 +51,7 @@ import (
5051
_ "google.golang.org/grpc/encoding/gzip" // Register gzip compression.
5152
"google.golang.org/grpc/keepalive"
5253
"google.golang.org/grpc/metadata"
54+
grpcstatus "google.golang.org/grpc/status"
5355
)
5456

5557
func init() {
@@ -132,6 +134,31 @@ func WithRateLimiter(limiter ratelimit.Interface) Option {
132134
// ErrRPCRecovered is returned when a panic is caught from an RPC.
133135
var ErrRPCRecovered = errors.DefineInternal("rpc_recovered", "Internal Server Error")
134136

137+
// sanitizingHTTPErrorHandler wraps runtime.DefaultHTTPErrorHandler to HTML-escape
138+
// gRPC status messages before they are serialized to JSON. This prevents reflected
139+
// XSS from user-supplied input that flows into error messages (e.g., field mask paths,
140+
// strconv.ParseBool parse errors, route-not-found paths).
141+
//
142+
// Note: ErrorDetailsToProto separately sanitizes error attributes in the details
143+
// payload. This handler covers the top-level "message" field which contains the
144+
// formatted error string with raw attribute values.
145+
func sanitizingHTTPErrorHandler(
146+
ctx context.Context,
147+
mux *runtime.ServeMux,
148+
marshaler runtime.Marshaler,
149+
w http.ResponseWriter,
150+
r *http.Request,
151+
err error,
152+
) {
153+
if st, ok := grpcstatus.FromError(err); ok {
154+
// Rebuild the status with escaped message while preserving details.
155+
pb := st.Proto()
156+
pb.Message = html.EscapeString(pb.Message)
157+
err = grpcstatus.ErrorProto(pb)
158+
}
159+
runtime.DefaultHTTPErrorHandler(ctx, mux, marshaler, w, r, err)
160+
}
161+
135162
// New returns a new RPC server with a set of middlewares.
136163
// The given context is used in some of the middlewares, the given server options are passed to gRPC
137164
//
@@ -234,7 +261,7 @@ func New(ctx context.Context, opts ...Option) *Server {
234261
server.ServeMux = runtime.NewServeMux(
235262
runtime.WithMarshalerOption("*", jsonpb.TTN()),
236263
runtime.WithMarshalerOption("text/event-stream", jsonpb.TTNEventStream()),
237-
runtime.WithErrorHandler(runtime.DefaultHTTPErrorHandler),
264+
runtime.WithErrorHandler(sanitizingHTTPErrorHandler),
238265
runtime.WithMetadata(func(ctx context.Context, req *http.Request) metadata.MD {
239266
md := rpcmetadata.MD{
240267
Host: req.Host,

pkg/rpcserver/rpcserver_test.go

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,10 @@ package rpcserver_test
1616

1717
import (
1818
"context"
19+
"encoding/json"
20+
"io"
21+
"net/http"
22+
"net/http/httptest"
1923
"runtime"
2024
"testing"
2125
"time"
@@ -173,3 +177,38 @@ func (s *mockServer) DownlinkQueuePush(ctx context.Context, req *ttnpb.DownlinkQ
173177
s.pushCtx, s.pushReq = ctx, req
174178
return ttnpb.Empty, nil
175179
}
180+
181+
func TestSanitizingHTTPErrorHandler(t *testing.T) {
182+
t.Parallel()
183+
a := assertions.New(t)
184+
ctx := test.Context()
185+
186+
logHandler := memory.New()
187+
logger := log.NewLogger(logHandler)
188+
ctx = log.NewContext(ctx, logger)
189+
190+
server := rpcserver.New(ctx)
191+
192+
// Request a non-existent gRPC-gateway route with XSS payload in the path.
193+
xssPath := `/api/v3/<script>alert("xss")</script>`
194+
r := httptest.NewRequest(http.MethodGet, xssPath, nil)
195+
r.Header.Set("Content-Type", "application/json")
196+
rec := httptest.NewRecorder()
197+
198+
server.ServeHTTP(rec, r)
199+
200+
res := rec.Result()
201+
body, _ := io.ReadAll(res.Body)
202+
203+
// Verify the response is valid JSON with no unescaped script tags.
204+
var resp map[string]any
205+
a.So(json.Unmarshal(body, &resp), should.BeNil)
206+
207+
// Check the message field if present.
208+
if msg, ok := resp["message"].(string); ok {
209+
a.So(msg, should.NotContainSubstring, "<script>")
210+
}
211+
212+
// Also verify no unescaped tags in the raw body.
213+
a.So(string(body), should.NotContainSubstring, "<script>")
214+
}

pkg/ttnpb/errors.go

Lines changed: 40 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ package ttnpb
1616

1717
import (
1818
"fmt"
19+
"html"
1920

2021
"go.thethings.network/lorawan-stack/v3/pkg/errors"
2122
"go.thethings.network/lorawan-stack/v3/pkg/goproto"
@@ -69,6 +70,44 @@ func (e errorDetails) Details() []proto.Message {
6970
return msgs
7071
}
7172

73+
// sanitizeAttributeValue HTML-escapes strings within attribute values
74+
// to prevent reflected XSS when error responses are rendered as HTML.
75+
// Returns new values without mutating the originals.
76+
func sanitizeAttributeValue(v any) any {
77+
switch val := v.(type) {
78+
case string:
79+
return html.EscapeString(val)
80+
case []string:
81+
escaped := make([]string, len(val))
82+
for i, s := range val {
83+
escaped[i] = html.EscapeString(s)
84+
}
85+
return escaped
86+
case []any:
87+
escaped := make([]any, len(val))
88+
for i, item := range val {
89+
escaped[i] = sanitizeAttributeValue(item)
90+
}
91+
return escaped
92+
case map[string]any:
93+
return sanitizeAttributes(val)
94+
default:
95+
return v
96+
}
97+
}
98+
99+
// sanitizeAttributes returns a new attribute map with all string values HTML-escaped.
100+
func sanitizeAttributes(attrs map[string]any) map[string]any {
101+
if len(attrs) == 0 {
102+
return attrs
103+
}
104+
sanitized := make(map[string]any, len(attrs))
105+
for k, v := range attrs {
106+
sanitized[html.EscapeString(k)] = sanitizeAttributeValue(v)
107+
}
108+
return sanitized
109+
}
110+
72111
func ErrorDetailsToProto(e errors.ErrorDetails) *ErrorDetails {
73112
pb := &ErrorDetails{
74113
Namespace: e.Namespace(),
@@ -77,7 +116,7 @@ func ErrorDetailsToProto(e errors.ErrorDetails) *ErrorDetails {
77116
CorrelationId: e.CorrelationID(),
78117
Code: e.Code(),
79118
}
80-
if attributes := e.PublicAttributes(); len(attributes) > 0 {
119+
if attributes := sanitizeAttributes(e.PublicAttributes()); len(attributes) > 0 {
81120
attributesStruct, err := goproto.Struct(attributes)
82121
if err != nil {
83122
panic(fmt.Sprintf("Failed to encode error attributes: %s", err)) // Likely a bug in ttn (invalid attribute type).

pkg/ttnpb/errors_test.go

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,9 +19,53 @@ import (
1919

2020
"github.com/smarty/assertions"
2121
"go.thethings.network/lorawan-stack/v3/pkg/errors"
22+
"go.thethings.network/lorawan-stack/v3/pkg/goproto"
23+
"go.thethings.network/lorawan-stack/v3/pkg/ttnpb"
2224
"go.thethings.network/lorawan-stack/v3/pkg/util/test/assertions/should"
2325
)
2426

27+
func TestErrorDetailsToProtoXSSSanitization(t *testing.T) {
28+
t.Parallel()
29+
a := assertions.New(t)
30+
31+
errDef := errors.Define(
32+
"test_xss_sanitization",
33+
"XSS Test Error",
34+
"path", "paths", "count",
35+
)
36+
37+
xssPayload := `<script>alert("xss")</script>`
38+
39+
errWithXSS := errDef.WithAttributes(
40+
"path", xssPayload,
41+
"paths", []string{xssPayload, "safe"},
42+
"count", 42,
43+
)
44+
45+
pb := ttnpb.ErrorDetailsToProto(errWithXSS)
46+
a.So(pb, should.NotBeNil)
47+
48+
// Recover attributes from the proto to verify sanitization.
49+
attrs, err := goproto.Map(pb.GetAttributes())
50+
a.So(err, should.BeNil)
51+
52+
// String attribute must be escaped.
53+
a.So(attrs["path"], should.NotContainSubstring, "<script>")
54+
a.So(attrs["path"], should.ContainSubstring, "&lt;script&gt;")
55+
56+
// []string attribute values must be escaped.
57+
if paths, ok := attrs["paths"].([]any); ok && len(paths) >= 2 {
58+
a.So(paths[0], should.NotContainSubstring, "<script>")
59+
a.So(paths[0], should.ContainSubstring, "&lt;script&gt;")
60+
a.So(paths[1], should.Equal, "safe")
61+
} else {
62+
t.Fatal("expected paths attribute to be a list with 2 elements")
63+
}
64+
65+
// Numeric attribute must pass through unchanged.
66+
a.So(attrs["count"], should.Equal, float64(42))
67+
}
68+
2569
func TestGRPCConversion(t *testing.T) {
2670
a := assertions.New(t)
2771

pkg/webhandlers/error_test.go

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
package webhandlers_test
1616

1717
import (
18+
"encoding/json"
1819
"io"
1920
"net/http"
2021
"net/http/httptest"
@@ -27,6 +28,38 @@ import (
2728
. "go.thethings.network/lorawan-stack/v3/pkg/webhandlers"
2829
)
2930

31+
func TestNotFoundXSSSanitization(t *testing.T) {
32+
t.Parallel()
33+
a := assertions.New(t)
34+
35+
xssPath := `/api/v3/<script>alert("xss")</script>`
36+
37+
r := httptest.NewRequest(http.MethodGet, xssPath, nil)
38+
rec := httptest.NewRecorder()
39+
40+
NotFound(rec, r)
41+
42+
res := rec.Result()
43+
body, _ := io.ReadAll(res.Body)
44+
45+
a.So(res.StatusCode, should.Equal, http.StatusNotFound)
46+
47+
// Decode the JSON to inspect the attribute value after JSON decoding.
48+
var resp map[string]any
49+
a.So(json.Unmarshal(body, &resp), should.BeNil)
50+
51+
// Extract the route attribute from the error details.
52+
details, _ := resp["details"].([]any) //nolint:revive
53+
a.So(len(details), should.BeGreaterThan, 0)
54+
detail, _ := details[0].(map[string]any) //nolint:revive
55+
attrs, _ := detail["attributes"].(map[string]any) //nolint:revive
56+
route, _ := attrs["route"].(string) //nolint:revive
57+
58+
// The attribute value must be HTML-escaped, not the raw XSS payload.
59+
a.So(route, should.NotContainSubstring, "<script>")
60+
a.So(route, should.ContainSubstring, "&lt;script&gt;")
61+
}
62+
3063
func TestErrorHandler(t *testing.T) {
3164
ctx, getError := NewContextWithErrorValue(test.Context())
3265

0 commit comments

Comments
 (0)