fix: check bounds on remote write receive when parsing symbolized metadata

Signed-off-by: bwplotka <bwplotka@gmail.com>
This commit is contained in:
bwplotka 2026-05-08 09:43:29 +01:00
parent 91c184a899
commit 38ccf6e7fe
6 changed files with 79 additions and 7 deletions

View File

@ -104,7 +104,10 @@ func printV2(req *writev2.Request) error {
if err != nil {
return err
}
m := ts.ToMetadata(req.Symbols)
m, err := ts.ToMetadata(req.Symbols)
if err != nil {
return err
}
fmt.Println(l, m)
for _, s := range ts.Samples {

View File

@ -14,6 +14,8 @@
package writev2
import (
"fmt"
"github.com/prometheus/common/model"
"github.com/prometheus/prometheus/model/exemplar"
@ -30,7 +32,7 @@ func (m TimeSeries) ToLabels(b *labels.ScratchBuilder, symbols []string) (labels
}
// ToMetadata return model metadata from timeseries' remote metadata.
func (m TimeSeries) ToMetadata(symbols []string) metadata.Metadata {
func (m TimeSeries) ToMetadata(symbols []string) (metadata.Metadata, error) {
typ := model.MetricTypeUnknown
switch m.Metadata.Type {
case Metadata_METRIC_TYPE_COUNTER:
@ -48,11 +50,17 @@ func (m TimeSeries) ToMetadata(symbols []string) metadata.Metadata {
case Metadata_METRIC_TYPE_STATESET:
typ = model.MetricTypeStateset
}
if int(m.Metadata.UnitRef) >= len(symbols) {
return metadata.Metadata{}, fmt.Errorf("metadata unit_ref %d outside of symbols table (size %d)", m.Metadata.UnitRef, len(symbols))
}
if int(m.Metadata.HelpRef) >= len(symbols) {
return metadata.Metadata{}, fmt.Errorf("metadata help_ref %d outside of symbols table (size %d)", m.Metadata.HelpRef, len(symbols))
}
return metadata.Metadata{
Type: typ,
Unit: symbols[m.Metadata.UnitRef],
Help: symbols[m.Metadata.HelpRef],
}
}, nil
}
// FromMetadataType transforms a Prometheus metricType into writev2 metricType.

View File

@ -130,9 +130,31 @@ func TestToMetadata(t *testing.T) {
} {
t.Run("", func(t *testing.T) {
ts := writev2.TimeSeries{Metadata: tc.input}
require.Equal(t, tc.expected, ts.ToMetadata(sym.Symbols()))
meta, err := ts.ToMetadata(sym.Symbols())
require.NoError(t, err)
require.Equal(t, tc.expected, meta)
})
}
t.Run("out of bounds unit ref", func(t *testing.T) {
ts := writev2.TimeSeries{Metadata: writev2.Metadata{UnitRef: 999}}
_, err := ts.ToMetadata(sym.Symbols())
require.Error(t, err)
require.Contains(t, err.Error(), "metadata unit_ref 999 outside of symbols table")
})
t.Run("out of bounds help ref", func(t *testing.T) {
ts := writev2.TimeSeries{Metadata: writev2.Metadata{HelpRef: 999}}
_, err := ts.ToMetadata(sym.Symbols())
require.Error(t, err)
require.Contains(t, err.Error(), "metadata help_ref 999 outside of symbols table")
})
t.Run("empty symbols table", func(t *testing.T) {
ts := writev2.TimeSeries{Metadata: writev2.Metadata{}}
_, err := ts.ToMetadata([]string{})
require.Error(t, err)
})
}
func TestToHistogram_Empty(t *testing.T) {

View File

@ -1216,7 +1216,10 @@ func v2RequestToWriteRequest(v2Req *writev2.Request) (*prompb.WriteRequest, erro
if err != nil {
return nil, fmt.Errorf("failed to convert metadata labels: %w", err)
}
metadata := rts.ToMetadata(v2Req.Symbols)
metadata, err := rts.ToMetadata(v2Req.Symbols)
if err != nil {
return nil, fmt.Errorf("failed to convert metadata: %w", err)
}
metricFamilyName := labels.String()

View File

@ -319,7 +319,12 @@ func (h *writeHandler) appendV2(app storage.Appender, req *writev2.Request, rs *
continue
}
m := ts.ToMetadata(req.Symbols)
m, err := ts.ToMetadata(req.Symbols)
if err != nil {
badRequestErrs = append(badRequestErrs, fmt.Errorf("parsing metadata for series %v: %w", ts.LabelsRefs, err))
samplesWithInvalidLabels += len(ts.Samples) + len(ts.Histograms)
continue
}
if h.enableTypeAndUnitLabels && (m.Type != model.MetricTypeUnknown || m.Unit != "") {
slb := labels.NewScratchBuilder(ls.Len() + 2) // +2 for __type__ and __unit__
ls.Range(func(l labels.Label) {

View File

@ -422,6 +422,36 @@ func TestRemoteWriteHandler_V2Message(t *testing.T) {
expectedCode: http.StatusBadRequest,
expectedRespBody: "parsing labels for series [1 999]: labelRefs 1 (name) = 999 (value) outside of symbols table (size 18)\n",
},
{
desc: "Partial write; first series with out-of-bounds metadata unit ref",
input: append(
[]writev2.TimeSeries{{
LabelsRefs: []uint32{1, 2},
Metadata: writev2.Metadata{
Type: writev2.Metadata_METRIC_TYPE_GAUGE,
UnitRef: 999,
},
Samples: []writev2.Sample{{Value: 1, Timestamp: 1}},
}},
writeV2RequestFixture.Timeseries...),
expectedCode: http.StatusBadRequest,
expectedRespBody: "parsing metadata for series [1 2]: metadata unit_ref 999 outside of symbols table (size 18)\n",
},
{
desc: "Partial write; first series with out-of-bounds metadata help ref",
input: append(
[]writev2.TimeSeries{{
LabelsRefs: []uint32{1, 2},
Metadata: writev2.Metadata{
Type: writev2.Metadata_METRIC_TYPE_GAUGE,
HelpRef: 999,
},
Samples: []writev2.Sample{{Value: 1, Timestamp: 1}},
}},
writeV2RequestFixture.Timeseries...),
expectedCode: http.StatusBadRequest,
expectedRespBody: "parsing metadata for series [1 2]: metadata help_ref 999 outside of symbols table (size 18)\n",
},
{
desc: "Partial write; TimeSeries with only exemplars (no samples or histograms)",
input: append(
@ -791,7 +821,8 @@ func TestRemoteWriteHandler_V2Message(t *testing.T) {
}
}
if tc.appendMetadata && tc.updateMetadataErr == nil {
expectedMeta := ts.ToMetadata(writeV2RequestFixture.Symbols)
expectedMeta, err := ts.ToMetadata(writeV2RequestFixture.Symbols)
require.NoError(t, err)
requireEqual(t, mockMetadata{ls, expectedMeta}, appendable.metadata[m])
m++
}