Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,10 @@ For details about compatibility between different releases, see the **Commitment

- 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.

### Security

- HTML-escape gRPC error messages and error detail attributes to prevent reflected XSS in API error responses.

## [3.35.2] - 2026-01-30

### Changed
Expand Down
29 changes: 28 additions & 1 deletion pkg/rpcserver/rpcserver.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package rpcserver
import (
"context"
"fmt"
"html"
"math"
"net/http"
"net/textproto"
Expand Down Expand Up @@ -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() {
Expand Down Expand Up @@ -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
//
Expand Down Expand Up @@ -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,
Expand Down
39 changes: 39 additions & 0 deletions pkg/rpcserver/rpcserver_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,10 @@ package rpcserver_test

import (
"context"
"encoding/json"
"io"
"net/http"
"net/http/httptest"
"runtime"
"testing"
"time"
Expand Down Expand Up @@ -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>")
}
41 changes: 40 additions & 1 deletion pkg/ttnpb/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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(),
Expand All @@ -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).
Expand Down
44 changes: 44 additions & 0 deletions pkg/ttnpb/errors_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
33 changes: 33 additions & 0 deletions pkg/webhandlers/error_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
package webhandlers_test

import (
"encoding/json"
"io"
"net/http"
"net/http/httptest"
Expand All @@ -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())

Expand Down
Loading