Skip to content

Querier: fix panic on invalid UTF-8 in active request tracker - #7743

Open
pujitha24 wants to merge 2 commits into
cortexproject:masterfrom
pujitha24:auto/issue-7729
Open

Querier: fix panic on invalid UTF-8 in active request tracker#7743
pujitha24 wants to merge 2 commits into
cortexproject:masterfrom
pujitha24:auto/issue-7729

Conversation

@pujitha24

Copy link
Copy Markdown
Contributor

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


AI assistance: this change was drafted with Claude Code.

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 cortexproject#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 cortexproject#7729

Signed-off-by: Pujitha Paladugu <10557236+pujitha24@users.noreply.github.com>
Comment thread CHANGELOG.md Outdated
* [BUGFIX] Querier: Fix gRPC `codes.Canceled` errors being mapped to HTTP 500 instead of 499 when a client cancels a query. #7738
* [BUGFIX] Compactor: Fix spurious `bucket operation fail after retries` error logs emitted during partial block cleanup. #7749
* [BUGFIX] Parquet Converter: Fix `auto_forget_delay` having no effect. The ring lifecycler was created without the auto-forget delegate, so unhealthy instances were never automatically removed from the ring. #7752
* [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

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you fix a PR number?

Signed-off-by: Pujitha Paladugu <10557236+pujitha24@users.noreply.github.com>
@pujitha24

Copy link
Copy Markdown
Contributor Author

Good catch, that was pointing at the issue number. Fixed it to reference #7743.

@SungJin1212 SungJin1212 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@dosubot dosubot Bot added the lgtm This PR has been approved by a maintainer label Aug 11, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

component/querier lgtm This PR has been approved by a maintainer size/S type/bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Querier crash: invalid UTF-8 in match[]/query panics the active request tracker (index out of range [-1])

2 participants