Skip to content

receive: validate remote write v2 symbol references - #8976

Open
r0h1tb wants to merge 3 commits into
thanos-io:mainfrom
r0h1tb:fix/receive-v2-symbol-bounds
Open

receive: validate remote write v2 symbol references#8976
r0h1tb wants to merge 3 commits into
thanos-io:mainfrom
r0h1tb:fix/receive-v2-symbol-bounds

Conversation

@r0h1tb

@r0h1tb r0h1tb commented Aug 12, 2026

Copy link
Copy Markdown

Changes

translateV2ToV1 resolves remote write v2 label references by indexing the request's
symbols table directly:

Name:  w.Symbols[t.LabelsRefs[i]],
Value: w.Symbols[t.LabelsRefs[i+1]],

Both the references and the table come from the client, so a payload whose
labels_refs point past the end of symbols panics the request goroutine with an
index out of range. Same pattern for exemplar refs. handleV2HTTP unmarshals and
calls the translation with no validation in between, so any v2 client can trigger it.
net/http recovers the panic per connection, so the process survives, but the
request is dropped and a stack trace is logged.

Prometheus enforces two invariants when desymbolizing and returns an error for
either — labels_refs must have even length, and every reference must address the
table (prompb/io/prometheus/write/v2.desymbolizeLabels). The v2 to v1 translation
reimplemented that lookup without them.

  • Add desymbolizeLabels with both checks and use it for series and exemplar labels.
  • translateV2ToV1 returns an error; handleV2HTTP maps it to 400.

Odd-length labels_refs previously dropped the trailing element silently; it is now
a 400, matching Prometheus.

Verification

Without the fix, a ref past the end panics:

--- FAIL: TestReproSymbolOutOfRange (0.00s)
panic: runtime error: index out of range [99] with length 2 [recovered, repanicked]
	pkg/receive/handler.go:568

translateV2ToV1 had no unit test, which is why this went unnoticed; the new file
covers the happy path and four malformed-reference cases.

go test -tags slicelabels -race ./pkg/receive/ passes and matches the baseline
(ok, 166s). golangci-lint run --build-tags=slicelabels ./pkg/receive/... reports
nothing for the changed or added files; the package's existing goconst findings are
unchanged.

  • I added CHANGELOG entry for this change.

translateV2ToV1 resolved label references by indexing the request's symbols
table directly. Both the references and the table come from the remote write
client, so a payload whose labels_refs point past the end of symbols panics
the request goroutine with an index out of range.

Prometheus enforces two invariants when desymbolizing (labels_refs must have
an even length, and every reference must address the table) and returns an
error for either. The v2 to v1 translation reimplemented that lookup without
them.

Reject both as bad requests instead of indexing blindly, and cover the
translation, which had no unit test.

Signed-off-by: Rohit Behera <126186063+r0h1tb@users.noreply.github.com>
Signed-off-by: Rohit Behera <126186063+r0h1tb@users.noreply.github.com>
@codecov

codecov Bot commented Aug 12, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 84.84848% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 63.39%. Comparing base (b453e37) to head (a2ec16a).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
pkg/receive/handler.go 84.84% 5 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #8976      +/-   ##
==========================================
- Coverage   64.54%   63.39%   -1.16%     
==========================================
  Files         289      289              
  Lines       37359    46004    +8645     
==========================================
+ Hits        24115    29166    +5051     
- Misses      11151    14740    +3589     
- Partials     2093     2098       +5     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

The snippet embeds pkg/receive/handler.go by line number and the symbol
validation added earlier in this PR shifted writeQuorum down, so make docs
regenerated the block with tryWrite instead.

Signed-off-by: Rohit Behera <126186063+r0h1tb@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant