sipcapture / heplify

Portable and Lightweight HEP Capture Agent for HOMER
https://sipcapture.org
GNU Affero General Public License v3.0
185 stars 65 forks source link

Potential bug #170

Open systemcrash opened 4 years ago

systemcrash commented 4 years ago

https://github.com/sipcapture/heplify/blob/1b2a80825bf4ef051337a8a746285346c7d992c3/decoder/correlator.go#L53

// Minimum RTCP port length of "m=audio 1000" = 12

Potentially, this is wrong. This assumes that only non-privileged ports are used by the declaring host. If the host declaring the SDP has port number 8 available to listen, the string is shorter. I have seen these in the wild.

Consider

// Minimum RTCP port length of "m=audio 8" = 9

i.e.

    if posRestPort := bytes.Index(restPort, []byte(" RTP")); posRestPort >= 9 {

I think.

systemcrash commented 4 years ago

Same assumption goes for:

https://github.com/sipcapture/heplify/blob/1b2a80825bf4ef051337a8a746285346c7d992c3/decoder/correlator.go#L42

systemcrash commented 4 years ago

These standards do not mandate port ranges or string lengths for the m= or a= row: https://tools.ietf.org/html/rfc4566#section-5.14

https://tools.ietf.org/html/rfc2327#page-19

Also, the assumption is that the media type is audio. Possible types are: https://tools.ietf.org/html/rfc2327#page-19

The first sub-field is the media type.  Currently defined media are
     "audio", "video", "application", "data" and "control", though this
     list may be extended as new communication modalities emerge (e.g.,
     telepresense).

I have seen text. So data or text = length 4.

making:

// Minimum RTCP port length of "m=data 8" = 8
systemcrash commented 4 years ago

But this is not a safe assumption. a media type of z is theoretically possible ;)

negbie commented 4 years ago

Yeah could be improved.