Skip to content

fix idx out of range in request tracker - #7640

Merged
danielblando merged 4 commits into
cortexproject:masterfrom
eeldaly:query-tracker-panic-fix
Jun 23, 2026
Merged

fix idx out of range in request tracker#7640
danielblando merged 4 commits into
cortexproject:masterfrom
eeldaly:query-tracker-panic-fix

Conversation

@eeldaly

@eeldaly eeldaly commented Jun 23, 2026

Copy link
Copy Markdown
Contributor

What this PR does:
Fixes an issue where we panic due to trying to truncate a multi-byte UTF-8 character in request tracker

Checklist

  • Tests updated
  • Documentation added
  • CHANGELOG.md updated - the order of entries should be [CHANGE], [FEATURE], [ENHANCEMENT], [BUGFIX]
  • docs/configuration/v1-guarantees.md updated if this PR introduces experimental flags

Signed-off-by: Essam Eldaly <eeldaly@amazon.com>
@dosubot dosubot Bot added go Pull requests that update Go code type/bug labels Jun 23, 2026
Signed-off-by: Essam Eldaly <eeldaly@amazon.com>
@danielblando

Copy link
Copy Markdown
Contributor

can you add a test case for it?

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

This looks solid. Would you mind including a test to cover this scenario?

eeldaly added 2 commits June 23, 2026 10:16
Signed-off-by: Essam Eldaly <eeldaly@amazon.com>
Signed-off-by: Essam Eldaly <eeldaly@amazon.com>
@danielblando
danielblando merged commit 4cc4166 into cortexproject:master Jun 23, 2026
37 checks passed
@eeldaly
eeldaly deleted the query-tracker-panic-fix branch June 23, 2026 18:15
pujitha24 added a commit to pujitha24/cortex that referenced this pull request Aug 8, 2026
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>
pujitha24 added a commit to pujitha24/cortex that referenced this pull request Aug 10, 2026
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>
pujitha24 added a commit to pujitha24/cortex that referenced this pull request Aug 17, 2026
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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

go Pull requests that update Go code size/M type/bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants