Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Potential bug #170

Open
systemcrash opened this issue May 15, 2020 · 4 comments
Open

Potential bug #170

systemcrash opened this issue May 15, 2020 · 4 comments

Comments

@systemcrash
Copy link

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

// 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
Copy link
Author

Same assumption goes for:

// Minimum RTCP port length of "a=rtcp:1000" = 11

@systemcrash
Copy link
Author

systemcrash commented May 15, 2020

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
Copy link
Author

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

@negbie
Copy link
Member

negbie commented May 15, 2020

Yeah could be improved.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants