From 0539a0ab9ce68f5309cc7b9d32efb78ab31348e6 Mon Sep 17 00:00:00 2001 From: Pujitha Paladugu <10557236+pujitha24@users.noreply.github.com> Date: Fri, 31 Jul 2026 23:53:30 -0700 Subject: [PATCH] Querier: fix panic on invalid UTF-8 in active request tracker MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Motivation: trimStringByBytes in the active request tracker scans backwards from a byte offset for a UTF-8 rune-start byte, with no lower bound. If a truncated value consists entirely of UTF-8 continuation bytes (0x80-0xBF), no byte in the string is ever a rune start, so the loop decrements past 0 and the subsequent slice index panics with "index out of range [-1]". The active request tracker is enabled by default (-querier.active-query-tracker-dir) and wraps the Prometheus API router, including /api/v1/series, /api/v1/labels and /api/v1/label/{name}/values. A tenant-supplied match[] or query value long enough to be truncated (over maxEntrySize) and made of invalid UTF-8 continuation bytes reaches trimStringByBytes byte-for-byte, so this is reachable from request input without special privileges. This is the same underflow class as #7640, which added a bound in trimForJsonMarshalRecursive but did not touch the scan inside trimStringByBytes; its regression tests only use valid multi-byte UTF-8, which always terminates the backwards scan before reaching index 0. This change only fixes the panic in trimStringByBytes. It does not add a recover() to the querier worker goroutine that runs request handling (pkg/querier/worker/scheduler_processor.go) — that was suggested in the issue as a separate, additional hardening measure and is out of scope for this fix. Approach: Bound the backwards scan with size > 0 so it stops at index 0 instead of underflowing. For any valid UTF-8 input, byte 0 is always a rune start, so this does not change behavior for legitimate input; it only changes the degenerate all-continuation-byte case, which now correctly trims to an empty string instead of panicking. Validation: - go build ./... - go test ./pkg/util/request_tracker/... (all pass, including the new TestTrimForJsonMarshalInvalidUTF8 regression test) - Confirmed TestTrimForJsonMarshalInvalidUTF8 reproduces the exact panic ("index out of range [-1]" at request_extractor.go:86) against the pre-fix code by temporarily reverting the one-line fix and re-running the test, then restored the fix and re-ran to confirm it passes. Fixes #7729 Signed-off-by: Pujitha Paladugu <10557236+pujitha24@users.noreply.github.com> --- CHANGELOG.md | 1 + pkg/util/request_tracker/request_extractor.go | 2 +- pkg/util/request_tracker/request_tracker_test.go | 14 ++++++++++++++ 3 files changed, 16 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6cc35782015..6c2100d65bd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -84,6 +84,7 @@ * [BUGFIX] Ring: Fix DynamoDB KV CAS not retrying on transactional conditional check failures. `TransactWriteItems` reports condition failures as `TransactionCanceledException` with a `ConditionalCheckFailed` cancellation reason, which was not recognized as retryable, so any concurrent ring update conflict (e.g. many ingesters joining during a rolling update) failed immediately instead of re-reading and retrying. `TransactionConflict` cancellation reasons are also treated as retryable. #7706 * [BUGFIX] Distributor: Return HTTP 499 (Client Closed Request) instead of 500 when a remote-write or OTLP push is canceled by the client, so client-side cancellations are no longer counted as server-side errors. #7717 * [BUGFIX] Querier: Fix gRPC `codes.Canceled` errors being mapped to HTTP 500 instead of 499 when a client cancels a query. #7738 +* [BUGFIX] Querier: Fix panic (`index out of range [-1]`) in the active request tracker when truncating a `match[]`/`query` value made entirely of invalid UTF-8 continuation bytes. The backwards scan for a rune boundary now stops at index 0 instead of underflowing. #7729 ## 1.21.1 2026-06-04 diff --git a/pkg/util/request_tracker/request_extractor.go b/pkg/util/request_tracker/request_extractor.go index cbd9f31e8fe..b9e7ff4d0f0 100644 --- a/pkg/util/request_tracker/request_extractor.go +++ b/pkg/util/request_tracker/request_extractor.go @@ -83,7 +83,7 @@ func trimStringByBytes(str string, size int) string { bytesStr := []byte(str) trimIndex := len(bytesStr) if size < len(bytesStr) { - for !utf8.RuneStart(bytesStr[size]) { + for size > 0 && !utf8.RuneStart(bytesStr[size]) { size-- } trimIndex = size diff --git a/pkg/util/request_tracker/request_tracker_test.go b/pkg/util/request_tracker/request_tracker_test.go index 13ef0a802cd..5cdc6fed2b5 100644 --- a/pkg/util/request_tracker/request_tracker_test.go +++ b/pkg/util/request_tracker/request_tracker_test.go @@ -161,6 +161,20 @@ func TestTrimForJsonMarshalMultiByteUTF8(t *testing.T) { } } +// TestTrimForJsonMarshalInvalidUTF8 reproduces a panic where a string made +// entirely of UTF-8 continuation bytes (0x80-0xBF, no valid rune-start byte) +// caused the backwards scan for a rune boundary to underflow past index 0 +// and index the byte slice with a negative index. +func TestTrimForJsonMarshalInvalidUTF8(t *testing.T) { + invalid := strings.Repeat("\x80", 1200) + + require.NotPanics(t, func() { + out := trimForJsonMarshal(invalid, 800) + assert.True(t, utf8.ValidString(out), "result should be valid UTF-8") + assert.Equal(t, "", out) + }) +} + // TestGenerateJSONEntryWithTruncatedFieldNegativeSize reproduces the request // tracker panic where a multi-byte UTF-8 field had to be truncated to a // negative remaining size because the rest of the entry already consumed the