From daecd971012dcc035cde2a697007df2bc2b37e56 Mon Sep 17 00:00:00 2001 From: philipch07 Date: Tue, 21 Oct 2025 18:02:20 -0400 Subject: [PATCH] Ignore malformed candidates --- unmarshal.go | 57 ++++++++++++++---- unmarshal_test.go | 149 ++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 196 insertions(+), 10 deletions(-) diff --git a/unmarshal.go b/unmarshal.go index d694ccb..41b0d4d 100644 --- a/unmarshal.go +++ b/unmarshal.go @@ -6,6 +6,7 @@ package sdp import ( "errors" "fmt" + "net" "net/url" "strconv" "strings" @@ -134,6 +135,24 @@ func (s *SessionDescription) UnmarshalString(value string) error { return nil } +// isBadCandidateAddress returns true if the candidate line has an invalid connection-address. +// It allows mDNS hostnames (".local") per ICE-mDNS. +func isBadCandidateAddress(candidateValue string) bool { + fields := strings.Fields(candidateValue) + + if len(fields) < 6 { + return true // clearly malformed candidate line + } + + addr := fields[4] + + if strings.HasSuffix(addr, ".local") { + return false // always allow mDNS host candidates + } + + return net.ParseIP(addr) == nil +} + // Unmarshal converts the value into a []byte and then calls UnmarshalString. // Callers should use the more performant UnmarshalString. func (s *SessionDescription) Unmarshal(value []byte) error { @@ -813,18 +832,27 @@ func unmarshalSessionEncryptionKey(l *lexer) (stateFn, error) { return s11, nil } -func unmarshalSessionAttribute(l *lexer) (stateFn, error) { - value, err := l.readLine() +func unmarshalSessionAttribute(lex *lexer) (stateFn, error) { + value, err := lex.readLine() if err != nil { return nil, err } i := strings.IndexRune(value, ':') - a := l.cache.getSessionAttribute() if i > 0 { - a.Key = value[:i] - a.Value = value[i+1:] + key := value[:i] + val := value[i+1:] + + if key == AttrKeyCandidate && isBadCandidateAddress(val) { + // Ignore malformed candidate; keep processing rest of SDP + return s11, nil + } + + a := lex.cache.getSessionAttribute() + a.Key = key + a.Value = val } else { + a := lex.cache.getSessionAttribute() a.Key = value } @@ -975,18 +1003,27 @@ func unmarshalMediaEncryptionKey(l *lexer) (stateFn, error) { return s14, nil } -func unmarshalMediaAttribute(l *lexer) (stateFn, error) { - value, err := l.readLine() +func unmarshalMediaAttribute(lex *lexer) (stateFn, error) { + value, err := lex.readLine() if err != nil { return nil, err } i := strings.IndexRune(value, ':') - a := l.cache.getMediaAttribute() if i > 0 { - a.Key = value[:i] - a.Value = value[i+1:] + key := value[:i] + val := value[i+1:] + + if key == AttrKeyCandidate && isBadCandidateAddress(val) { + // Ignore malformed candidate; keep processing rest of SDP + return s14, nil + } + + a := lex.cache.getMediaAttribute() + a.Key = key + a.Value = val } else { + a := lex.cache.getMediaAttribute() a.Key = value } diff --git a/unmarshal_test.go b/unmarshal_test.go index 3feefbc..da83c1c 100644 --- a/unmarshal_test.go +++ b/unmarshal_test.go @@ -5,7 +5,10 @@ package sdp import ( "io" + "net" + "strings" "testing" + "unicode" "github.com/stretchr/testify/assert" ) @@ -174,6 +177,11 @@ const ( MediaTCPTLSMRCPv2 = TimingSDP + "m=application 1544 TCP/TLS/MRCPv2 1\r\n" + minimalBaseSDP = "v=0\r\n" + + "o=- 1 1 IN IP4 0.0.0.0\r\n" + + "s=-\r\n" + + "t=0 0\r\n" + CanonicalUnmarshalSDP = "v=0\r\n" + "o=jdoe 2890844526 2890842807 IN IP4 10.47.16.5\r\n" + "s=SDP Seminar\r\n" + @@ -1901,3 +1909,144 @@ func TestTimeShorthand_MinutesAndSeconds(t *testing.T) { assert.Equal(t, int64(1), timeShorthand('s')) }) } + +func TestIgnoreMalformedSessionCandidate(t *testing.T) { + in := minimalBaseSDP + + "a=recvonly\r\n" + + // invalid IPv4 address should be ignored + "a=candidate:1 1 UDP 12345 999.999.999.999 1234 typ host\r\n" + + var sd SessionDescription + err := sd.UnmarshalString(in) + assert.NoError(t, err) + + out, err := sd.Marshal() + assert.NoError(t, err) + + // candidate line should be dropped, everything else is preserved + want := minimalBaseSDP + "a=recvonly\r\n" + assert.Equal(t, want, string(out)) +} + +func TestIgnoreMalformedMediaCandidate(t *testing.T) { + in := minimalBaseSDP + + "m=audio 9 RTP/AVP 0\r\n" + + "a=rtpmap:0 PCMU/8000\r\n" + + // malformed, should be ignored + "a=candidate:1 1 UDP 12345 not_an_ip 1234 typ host\r\n" + + // valid candidate remains + "a=candidate:2 1 UDP 12345 203.0.113.1 2345 typ host\r\n" + + var sd SessionDescription + err := sd.UnmarshalString(in) + assert.NoError(t, err) + + out, err := sd.Marshal() + assert.NoError(t, err) + + want := minimalBaseSDP + + "m=audio 9 RTP/AVP 0\r\n" + + "a=rtpmap:0 PCMU/8000\r\n" + + "a=candidate:2 1 UDP 12345 203.0.113.1 2345 typ host\r\n" + assert.Equal(t, want, string(out)) +} + +func TestAllowMDNSCandidate(t *testing.T) { + in := minimalBaseSDP + + // mDNS hostnames are valid + "a=candidate:1 1 UDP 12345 myhost.local 1234 typ host\r\n" + + var sd SessionDescription + err := sd.UnmarshalString(in) + assert.NoError(t, err) + + out, err := sd.Marshal() + assert.NoError(t, err) + assert.Equal(t, in, string(out)) +} + +func TestAllowIPv6Candidate(t *testing.T) { + in := minimalBaseSDP + + // IPv6 literal is valid + "a=candidate:1 1 UDP 12345 2001:db8::1 5555 typ host\r\n" + + var sd SessionDescription + err := sd.UnmarshalString(in) + assert.NoError(t, err) + + out, err := sd.Marshal() + assert.NoError(t, err) + assert.Equal(t, in, string(out)) +} + +func TestIgnoreTruncatedCandidate(t *testing.T) { + in := minimalBaseSDP + + // missing port + fewer than 6 fields after split should be ignored + "a=candidate:1 1 UDP 2113667327 203.0.113.1\r\n" + + var sd SessionDescription + err := sd.UnmarshalString(in) + assert.NoError(t, err) + + out, err := sd.Marshal() + assert.NoError(t, err) + // attribute should be dropped + assert.Equal(t, minimalBaseSDP, string(out)) +} + +func FuzzUnmarshalIgnoreMalformedCandidates(f *testing.F) { + // Seeds: valid IPv4, valid IPv6, mDNS, and some bad cases + for _, seed := range []string{ + "203.0.113.1", + "2001:db8::1", + "router.local", + "999.999.999.999", + "bad space", // should be sanitized + "", // should be coerced + "::::", // invalid IPv6 + "host_name", // underscore is invalid for DNS but we just treat as string + "-.-", // sample punctuation + } { + f.Add(seed) + } + + f.Fuzz(func(t *testing.T, addr string) { + // sanitize fuzzed addr to avoid breaking SDP tokenization/lines + addr = strings.Map(func(r rune) rune { + switch { + case unicode.IsLetter(r), unicode.IsDigit(r): + return r + } + // allow a few safe punctuation characters + switch r { + case '.', ':', '-': + return r + default: + return -1 + } + }, addr) + if addr == "" { + addr = "bad" + } + + line := "a=candidate:1 1 UDP 12345 " + addr + " 1 typ host\r\n" + in := minimalBaseSDP + line + + var sd SessionDescription + err := sd.UnmarshalString(in) + // unmarshal should never error if a candidate address is malformed. + assert.NoError(t, err) + + outBytes, err := sd.Marshal() + assert.NoError(t, err) + out := string(outBytes) + + // Determine if candidate should be kept + keep := strings.HasSuffix(addr, ".local") || net.ParseIP(addr) != nil + if keep { + assert.Contains(t, out, line) + } else { + assert.NotContains(t, out, line) + } + }) +}