From 0759280cd018b8f588ca0ca6642caba3d60b5bcf Mon Sep 17 00:00:00 2001 From: Felix-Ayush <67006255+Ayush7614@users.noreply.github.com> Date: Thu, 30 Jul 2026 15:24:50 +0530 Subject: [PATCH] Fix getRefLayerRTPTimestamp off-by-one that can panic on max layer index (#4712) * Fix getRefLayerRTPTimestamp off-by-one that can panic on max layer index. Reject ref/target layers with >= len(refInfos) so layer==len is an error instead of an out-of-range index. * Remove historical comment from ref-layer bounds test. Keep the regression coverage without referencing the old bounds check in source. --- pkg/sfu/forwarder.go | 2 +- pkg/sfu/forwarder_test.go | 28 ++++++++++++++++++++++++++++ 2 files changed, 29 insertions(+), 1 deletion(-) diff --git a/pkg/sfu/forwarder.go b/pkg/sfu/forwarder.go index 03cc46df2..527d5165f 100644 --- a/pkg/sfu/forwarder.go +++ b/pkg/sfu/forwarder.go @@ -1792,7 +1792,7 @@ func (f *Forwarder) GetTranslationParams(extPkt *buffer.ExtPacket, layer int32) } func (f *Forwarder) getRefLayerRTPTimestamp(ts uint32, refLayer, targetLayer int32) (uint32, error) { - if refLayer < 0 || int(refLayer) > len(f.refInfos) || targetLayer < 0 || int(targetLayer) > len(f.refInfos) { + if refLayer < 0 || int(refLayer) >= len(f.refInfos) || targetLayer < 0 || int(targetLayer) >= len(f.refInfos) { return 0, fmt.Errorf("invalid layer(s), refLayer: %d, targetLayer: %d", refLayer, targetLayer) } diff --git a/pkg/sfu/forwarder_test.go b/pkg/sfu/forwarder_test.go index 513fbe40e..ff83d61a6 100644 --- a/pkg/sfu/forwarder_test.go +++ b/pkg/sfu/forwarder_test.go @@ -2187,3 +2187,31 @@ func TestForwarderInitialAcquisitionGrace(t *testing.T) { require.Equal(t, int32(1), alloc.TargetLayer.Spatial) require.Equal(t, int32(1), alloc.RequestLayerSpatial) } + +func TestGetRefLayerRTPTimestampBounds(t *testing.T) { + f := newForwarder(testutils.TestVP8Codec, webrtc.RTPCodecTypeVideo) + + layerCount := int32(len(f.refInfos)) + + _, err := f.getRefLayerRTPTimestamp(1000, layerCount, 0) + require.Error(t, err) + + _, err = f.getRefLayerRTPTimestamp(1000, 0, layerCount) + require.Error(t, err) + + _, err = f.getRefLayerRTPTimestamp(1000, -1, 0) + require.Error(t, err) + + _, err = f.getRefLayerRTPTimestamp(1000, 0, -1) + require.Error(t, err) + + // same-layer translation does not need sender reports + ts, err := f.getRefLayerRTPTimestamp(1000, 0, 0) + require.NoError(t, err) + require.Equal(t, uint32(1000), ts) + + // last valid index must be accepted by the bounds check (may still error for missing SR) + _, err = f.getRefLayerRTPTimestamp(1000, layerCount-1, 0) + require.Error(t, err) // unavailable sender report, not invalid layer + require.Contains(t, err.Error(), "unavailable") +}